JAL-2490 tidy code and tests for feature export in Jalview format
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Tue, 23 May 2017 11:37:10 +0000 (12:37 +0100)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Tue, 23 May 2017 11:37:10 +0000 (12:37 +0100)
src/jalview/appletgui/AlignFrame.java
src/jalview/datamodel/features/SequenceFeatures.java
src/jalview/gui/AnnotationExporter.java
src/jalview/io/FeaturesFile.java
test/jalview/datamodel/features/SequenceFeaturesTest.java
test/jalview/io/FeaturesFileTest.java

index f914108..b8700e1 100644 (file)
@@ -1447,12 +1447,12 @@ public class AlignFrame extends EmbmenuFrame implements ActionListener,
     if (format.equalsIgnoreCase("Jalview"))
     {
       features = formatter.printJalviewFormat(viewport.getAlignment()
-              .getSequencesArray(), getDisplayedFeatureCols());
+              .getSequencesArray(), getDisplayedFeatureCols(), true);
     }
     else
     {
       features = formatter.printGffFormat(viewport.getAlignment()
-              .getSequencesArray(), getDisplayedFeatureCols());
+              .getSequencesArray(), getDisplayedFeatureCols(), true);
     }
 
     if (displayTextbox)
index 34470cc..ff570b1 100644 (file)
@@ -220,9 +220,11 @@ public class SequenceFeatures implements SequenceFeaturesI
     /*
      * else make a copy of the list, and remove any null value just in case,
      * as it would cause errors looking up the features Map
+     * sort in alphabetical order for consistent output behaviour
      */
     List<String> types = new ArrayList<String>(Arrays.asList(type));
     types.remove(null);
+    Collections.sort(types);
     return types;
   }
 
index cffc52f..a48c030 100644 (file)
@@ -163,13 +163,13 @@ public class AnnotationExporter extends JPanel
       boolean includeNonPositional = ap.av.isShowNPFeats();
       if (GFFFormat.isSelected())
       {
-        text = formatter.printGffFormat(sequences, featureColours, true,
+        text = formatter.printGffFormat(sequences, featureColours,
                 includeNonPositional);
       }
       else
       {
         text = formatter.printJalviewFormat(sequences, featureColours,
-                true, includeNonPositional);
+                includeNonPositional);
       }
     }
     else
index 028b14f..869b18b 100755 (executable)
@@ -497,43 +497,24 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
     {
       sf.addLink(link);
     }
-
   }
 
   /**
-   * generate a features file for seqs includes non-pos features by default.
-   * 
-   * @param sequences
-   *          source of sequence features
-   * @param visible
-   *          hash of feature types and colours
-   * @return features file contents
-   */
-  public String printJalviewFormat(SequenceI[] sequences,
-          Map<String, FeatureColourI> visible)
-  {
-    return printJalviewFormat(sequences, visible, true, true);
-  }
-
-  /**
-   * generate a features file for seqs with colours from visible (if any)
+   * Returns contents of a Jalview format features file
    * 
    * @param sequences
    *          source of features
    * @param visible
-   *          hash of Colours for each feature type
-   * @param visOnly
-   *          when true only feature types in 'visible' will be output
-   * @param nonpos
-   *          indicates if non-positional features should be output (regardless
-   *          of group or type)
-   * @return features file contents
+   *          map of colour for each visible feature type
+   * @param includeNonPositional
+   *          if true, include non-positional features (regardless of group or
+   *          type)
+   * @return
    */
   public String printJalviewFormat(SequenceI[] sequences,
-          Map<String, FeatureColourI> visible, boolean visOnly,
-          boolean nonpos)
+          Map<String, FeatureColourI> visible, boolean includeNonPositional)
   {
-    if (visOnly && !nonpos && (visible == null || visible.isEmpty()))
+    if (!includeNonPositional && (visible == null || visible.isEmpty()))
     {
       // no point continuing.
       return "No Features Visible";
@@ -544,7 +525,7 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
      */
     // TODO: decide if feature links should also be written here ?
     StringBuilder out = new StringBuilder(256);
-    if (visible != null && visOnly) // todo why visOnly test?
+    if (visible != null)
     {
       for (Entry<String, FeatureColourI> featureColour : visible.entrySet())
       {
@@ -556,14 +537,14 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
 
     // Work out which groups are both present and visible
     Set<String> groups = new HashSet<String>();
-    String[] types = visible == null ? null : visible.keySet().toArray(
-            new String[visible.keySet().size()]);
+    String[] types = visible == null ? new String[0] : visible.keySet()
+            .toArray(new String[visible.keySet().size()]);
 
     for (int i = 0; i < sequences.length; i++)
     {
       groups.addAll(sequences[i].getFeatures()
               .getFeatureGroups(true, types));
-      if (nonpos)
+      if (includeNonPositional)
       {
         groups.addAll(sequences[i].getFeatures().getFeatureGroups(false,
                 types));
@@ -594,13 +575,16 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
       for (int i = 0; i < sequences.length; i++)
       {
         List<SequenceFeature> features = new ArrayList<SequenceFeature>();
-        if (nonpos)
+        if (includeNonPositional)
         {
           features.addAll(sequences[i].getFeatures().getFeaturesForGroup(
                   false, group, types));
         }
-        features.addAll(sequences[i].getFeatures().getFeaturesForGroup(
-                true, group, types));
+        if (types.length > 0)
+        {
+          features.addAll(sequences[i].getFeatures().getFeaturesForGroup(
+                  true, group, types));
+        }
 
         for (SequenceFeature sequenceFeature : features)
         {
@@ -722,34 +706,17 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
   }
 
   /**
-   * Returns features output in GFF2 format, including hidden and non-positional
-   * features
-   * 
-   * @param sequences
-   *          the sequences whose features are to be output
-   * @param visible
-   *          a map whose keys are the type names of visible features
-   * @return
-   */
-  public String printGffFormat(SequenceI[] sequences,
-          Map<String, FeatureColourI> visible)
-  {
-    return printGffFormat(sequences, visible, true, true);
-  }
-
-  /**
    * Returns features output in GFF2 format
    * 
    * @param sequences
    *          the sequences whose features are to be output
    * @param visible
    *          a map whose keys are the type names of visible features
-   * @param outputVisibleOnly
    * @param includeNonPositionalFeatures
    * @return
    */
   public String printGffFormat(SequenceI[] sequences,
-          Map<String, FeatureColourI> visible, boolean outputVisibleOnly,
+          Map<String, FeatureColourI> visible,
           boolean includeNonPositionalFeatures)
   {
     StringBuilder out = new StringBuilder(256);
@@ -772,10 +739,7 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
              */
             continue;
           }
-          // TODO why the test !isnonpos here?
-          // what about not visible non-positional features?
-          if (!isnonpos && outputVisibleOnly
-                  && !visible.containsKey(sf.type))
+          if (!isnonpos && !visible.containsKey(sf.type))
           {
             /*
              * ignore not visible features if not wanted
index d3927fe..eaa5276 100644 (file)
@@ -896,14 +896,14 @@ public class SequenceFeaturesTest
     assertFalse(iterator.hasNext());
 
     /*
-     * two types specified
+     * two types specified - get sorted alphabetically
      */
     types = sf.varargToTypes("Metal", "Helix");
     iterator = types.iterator();
     assertTrue(iterator.hasNext());
-    assertEquals(iterator.next(), "Metal");
-    assertTrue(iterator.hasNext());
     assertEquals(iterator.next(), "Helix");
+    assertTrue(iterator.hasNext());
+    assertEquals(iterator.next(), "Metal");
     assertFalse(iterator.hasNext());
 
     /*
@@ -912,9 +912,9 @@ public class SequenceFeaturesTest
     types = sf.varargToTypes("Metal", null, "Helix");
     iterator = types.iterator();
     assertTrue(iterator.hasNext());
-    assertEquals(iterator.next(), "Metal");
-    assertTrue(iterator.hasNext());
     assertEquals(iterator.next(), "Helix");
+    assertTrue(iterator.hasNext());
+    assertEquals(iterator.next(), "Metal");
     assertFalse(iterator.hasNext());
   }
 
index be5e68d..70a5f16 100644 (file)
@@ -421,33 +421,31 @@ public class FeaturesFileTest
     FeatureRenderer fr = af.alignPanel.getFeatureRenderer();
     Map<String, FeatureColourI> visible = fr.getDisplayedFeatureCols();
     String exported = featuresFile.printJalviewFormat(
-            al.getSequencesArray(), visible, true, false);
+            al.getSequencesArray(), visible, false);
     String expected = "No Features Visible";
     assertEquals(expected, exported);
 
     /*
      * include non-positional features
      */
-    af.getViewport().setShowNPFeats(true);
     exported = featuresFile.printJalviewFormat(al.getSequencesArray(),
-            visible, true, true);
+            visible, true);
     expected = "\nSTARTGROUP\tuniprot\nCath\tFER_CAPAA\t-1\t0\t0\tDomain\t0.0\nENDGROUP\tuniprot\n";
     assertEquals(expected, exported);
 
     /*
      * set METAL (in uniprot group) and GAMMA-TURN visible, but not Pfam
      */
-    af.getViewport().setShowNPFeats(false);
     fr.setVisible("METAL");
     fr.setVisible("GAMMA-TURN");
     visible = fr.getDisplayedFeatureCols();
     exported = featuresFile.printJalviewFormat(al.getSequencesArray(),
-            visible, true, false);
+            visible, false);
     expected = "METAL\tcc9900\n"
             + "GAMMA-TURN\tff0000|00ffff|20.0|95.0|below|66.0\n"
             + "\nSTARTGROUP\tuniprot\n"
-            + "Iron\tFER_CAPAA\t-1\t39\t39\tMETAL\t0.0\n"
             + "Turn\tFER_CAPAA\t-1\t36\t38\tGAMMA-TURN\t0.0\n"
+            + "Iron\tFER_CAPAA\t-1\t39\t39\tMETAL\t0.0\n"
             + "ENDGROUP\tuniprot\n";
     assertEquals(expected, exported);
 
@@ -457,17 +455,16 @@ public class FeaturesFileTest
     fr.setVisible("Pfam");
     visible = fr.getDisplayedFeatureCols();
     exported = featuresFile.printJalviewFormat(al.getSequencesArray(),
-            visible, true, false);
+            visible, false);
     /*
-     * note the order of feature types is uncontrolled - derives from
-     * FeaturesDisplayed.featuresDisplayed which is a HashSet
+     * features are output within group, ordered by sequence and by type
      */
     expected = "METAL\tcc9900\n"
             + "Pfam\tff0000\n"
             + "GAMMA-TURN\tff0000|00ffff|20.0|95.0|below|66.0\n"
             + "\nSTARTGROUP\tuniprot\n"
-            + "Iron\tFER_CAPAA\t-1\t39\t39\tMETAL\t0.0\n"
             + "Turn\tFER_CAPAA\t-1\t36\t38\tGAMMA-TURN\t0.0\n"
+            + "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";
     assertEquals(expected, exported);