JAL-3178 include non-positional features in their group on export

diff --git a/src/jalview/io/FeaturesFile.java b/src/jalview/io/FeaturesFile.java
index 39ec10d..12ad0d4 100755 (executable)
--- a/src/jalview/io/FeaturesFile.java
+++ b/src/jalview/io/FeaturesFile.java
@@ -581,7 +581,6 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
     Map<String, FeatureColourI> visibleColours = fr
             .getDisplayedFeatureCols();
     Map<String, FeatureMatcherSetI> featureFilters = fr.getFeatureFilters();
-    List<String> visibleFeatureGroups = fr.getDisplayedFeatureGroups();
 
     if (!includeNonPositional
             && (visibleColours == null || visibleColours.isEmpty()))
@@ -616,42 +615,12 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
     outputFeatureFilters(out, visibleColours, featureFilters);
 
     /*
-     * sort groups alphabetically, and ensure that features with a
-     * null or empty group are output after those in named groups
-     */
-    List<String> sortedGroups = new ArrayList<>(visibleFeatureGroups);
-    sortedGroups.remove(null);
-    sortedGroups.remove("");
-    Collections.sort(sortedGroups);
-    sortedGroups.add(null);
-    sortedGroups.add("");
-
-    boolean foundSome = false;
-
-    /*
-     * first output any non-positional features
-     */
-    if (includeNonPositional)
-    {
-      for (int i = 0; i < sequences.length; i++)
-      {
-        String sequenceName = sequences[i].getName();
-        for (SequenceFeature feature : sequences[i].getFeatures()
-                .getNonPositionalFeatures())
-        {
-          foundSome = true;
-          out.append(formatJalviewFeature(sequenceName, feature));
-        }
-      }
-    }
-
-    /*
-     * positional features within groups
+     * output features within groups
      */
-    foundSome |= outputFeaturesByGroup(out, fr, sortedGroups, types,
-            sequences);
+    int count = outputFeaturesByGroup(out, fr, types, sequences,
+            includeNonPositional);
 
-    return foundSome ? out.toString() : "No Features Visible";
+    return count > 0 ? out.toString() : "No Features Visible";
   }
 
   /**
@@ -697,61 +666,96 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
   /**
    * Appends output of visible sequence features within feature groups to the
    * output buffer. Groups other than the null or empty group are sandwiched by
-   * STARTGROUP and ENDGROUP lines. Answers true if at least one feature was
-   * written, else false.
+   * STARTGROUP and ENDGROUP lines. Answers the number of features written.
    * 
    * @param out
    * @param fr
-   * @param groups
    * @param featureTypes
    * @param sequences
+   * @param includeNonPositional
    * @return
    */
-  private boolean outputFeaturesByGroup(StringBuilder out,
-          FeatureRenderer fr, List<String> groups, String[] featureTypes,
-          SequenceI[] sequences)
+  private int outputFeaturesByGroup(StringBuilder out,
+          FeatureRenderer fr, String[] featureTypes,
+          SequenceI[] sequences, boolean includeNonPositional)
   {
-    boolean foundSome = false;
-    for (String group : groups)
+    List<String> featureGroups = fr.getFeatureGroups();
+
+    /*
+     * sort groups alphabetically, and ensure that features with a
+     * null or empty group are output after those in named groups
+     */
+    List<String> sortedGroups = new ArrayList<>(featureGroups);
+    sortedGroups.remove(null);
+    sortedGroups.remove("");
+    Collections.sort(sortedGroups);
+    sortedGroups.add(null);
+    sortedGroups.add("");
+
+    int count = 0;
+    List<String> visibleGroups = fr.getDisplayedFeatureGroups();
+
+    /*
+     * loop over all groups (may be visible or not);
+     * non-positional features are output even if group is not visible
+     */
+    for (String group : sortedGroups)
     {
       boolean firstInGroup = true;
-      boolean isNamedGroup = (group != null && !"".equals(group));
+      boolean isNullGroup = group == null || "".equals(group);
 
-      /*
-       * output positional features within groups
-       */
       for (int i = 0; i < sequences.length; i++)
       {
         String sequenceName = sequences[i].getName();
         List<SequenceFeature> features = new ArrayList<>();
-        if (featureTypes.length > 0)
+
+        /*
+         * get any non-positional features in this group, if wanted
+         * (for any feature type, whether visible or not)
+         */
+        if (includeNonPositional)
+        {
+          features.addAll(sequences[i].getFeatures()
+                  .getFeaturesForGroup(false, group));
+        }
+
+        /*
+         * add positional features for visible feature types, but
+         * (for named groups) only if feature group is visible
+         */
+        if (featureTypes.length > 0
+                && (isNullGroup || visibleGroups.contains(group)))
         {
           features.addAll(sequences[i].getFeatures().getFeaturesForGroup(
                   true, group, featureTypes));
         }
 
-        for (SequenceFeature sequenceFeature : features)
+        for (SequenceFeature sf : features)
         {
-          if (fr.isVisible(sequenceFeature))
+          if (sf.isNonPositional() || fr.isVisible(sf))
           {
-            foundSome = true;
-            if (firstInGroup && isNamedGroup)
+            count++;
+            if (firstInGroup)
             {
-              out.append(newline).append(STARTGROUP).append(TAB)
-                      .append(group).append(newline);
+              out.append(newline);
+              if (!isNullGroup)
+              {
+                out.append(STARTGROUP).append(TAB).append(group)
+                        .append(newline);
+              }
             }
             firstInGroup = false;
-            out.append(formatJalviewFeature(sequenceName, sequenceFeature));
+            out.append(formatJalviewFeature(sequenceName, sf));
           }
         }
       }
 
-      if (isNamedGroup && !firstInGroup)
+      if (!isNullGroup && !firstInGroup)
       {
         out.append(ENDGROUP).append(TAB).append(group).append(newline);
       }
     }
-    return foundSome;
+    return count;
   }
 
   /**

diff --git a/test/jalview/io/FeaturesFileTest.java b/test/jalview/io/FeaturesFileTest.java
index ab07289..3632cc7 100644 (file)
--- a/test/jalview/io/FeaturesFileTest.java
+++ b/test/jalview/io/FeaturesFileTest.java
@@ -481,13 +481,15 @@ public class FeaturesFileTest
     assertEquals(expected, exported);
 
     /*
-     * include non-positional features
+     * include non-positional features, but still no positional features
      */
     fr.setGroupVisibility("uniprot", true);
     exported = featuresFile.printJalviewFormat(al.getSequencesArray(), fr,
             true);
-    expected = "Cath\tFER_CAPAA\t-1\t0\t0\tDomain\t0.0\n"
-            + "desc1\tFER_CAPAN\t-1\t0\t0\tPfam\t1.3\n"
+    expected = "\nSTARTGROUP\tuniprot\n"
+            + "Cath\tFER_CAPAA\t-1\t0\t0\tDomain\t0.0\n"
+            + "ENDGROUP\tuniprot\n\n"
+            + "desc1\tFER_CAPAN\t-1\t0\t0\tPfam\t1.3\n\n"
             + "desc3\tFER1_SOLLC\t-1\t0\t0\tPfam\n"; // NaN is not output
     assertEquals(expected, exported);
 
@@ -513,7 +515,7 @@ public class FeaturesFileTest
     exported = featuresFile.printJalviewFormat(al.getSequencesArray(), fr,
             false);
     /*
-     * features are output within group, ordered by sequence and by type
+     * features are output within group, ordered by sequence and type
      */
     expected = "METAL\tcc9900\n"
             + "Pfam\tff0000\n"
@@ -523,9 +525,36 @@ public class FeaturesFileTest
             + "Iron\tFER_CAPAA\t-1\t39\t39\tMETAL\t0.0\n"
             + "<html>Pfam domain<a href=\"http://pfam.xfam.org/family/PF00111\">Pfam_3_4</a></html>\tFER_CAPAA\t-1\t20\t20\tPfam\t0.0\n"
             + "ENDGROUP\tuniprot\n"
-            // null / empty group features output after features in named
-            // groups:
+            // null / empty group features are output after named groups
+            + "\ndesc2\tFER_CAPAN\t-1\t4\t9\tPfam\n"
+            + "\ndesc4\tFER1_SOLLC\t-1\t5\t8\tPfam\t-2.6\n";
+    assertEquals(expected, exported);
+
+    /*
+     * hide uniprot group
+     */
+    fr.setGroupVisibility("uniprot", false);
+    expected = "METAL\tcc9900\n" + "Pfam\tff0000\n"
+            + "GAMMA-TURN\tscore|ff0000|00ffff|noValueMin|20.0|95.0|below|66.0\n"
+            + "\ndesc2\tFER_CAPAN\t-1\t4\t9\tPfam\n"
+            + "\ndesc4\tFER1_SOLLC\t-1\t5\t8\tPfam\t-2.6\n";
+    exported = featuresFile.printJalviewFormat(al.getSequencesArray(), fr,
+            false);
+    assertEquals(expected, exported);
+
+    /*
+     * include non-positional (overrides group not shown)
+     */
+    exported = featuresFile.printJalviewFormat(al.getSequencesArray(), fr,
+            true);
+    expected = "METAL\tcc9900\n" + "Pfam\tff0000\n"
+            + "GAMMA-TURN\tscore|ff0000|00ffff|noValueMin|20.0|95.0|below|66.0\n"
+            + "\nSTARTGROUP\tuniprot\n"
+            + "Cath\tFER_CAPAA\t-1\t0\t0\tDomain\t0.0\n"
+            + "ENDGROUP\tuniprot\n"
+            + "\ndesc1\tFER_CAPAN\t-1\t0\t0\tPfam\t1.3\n"
             + "desc2\tFER_CAPAN\t-1\t4\t9\tPfam\n"
+            + "\ndesc3\tFER1_SOLLC\t-1\t0\t0\tPfam\n"
             + "desc4\tFER1_SOLLC\t-1\t5\t8\tPfam\t-2.6\n";
     assertEquals(expected, exported);
   }