JAL-2490 improved lookup of features for Export Features (Jalview

format)

diff --git a/src/jalview/io/FeaturesFile.java b/src/jalview/io/FeaturesFile.java
index 2ce72ae..028b14f 100755 (executable)
--- a/src/jalview/io/FeaturesFile.java
+++ b/src/jalview/io/FeaturesFile.java
@@ -44,11 +44,14 @@ import java.awt.Color;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
-import java.util.Iterator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 
 /**
  * Parses and writes features files, which may be in Jalview, GFF2 or GFF3
@@ -76,6 +79,19 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
 
   protected static final String GFF_VERSION = "##gff-version";
 
+  private static final Comparator<String> SORT_NULL_LAST = new Comparator<String>()
+  {
+    @Override
+    public int compare(String o1, String o2)
+    {
+      if (o1 == null)
+      {
+        return o2 == null ? 0 : 1;
+      }
+      return (o2 == null ? -1 : o1.compareTo(o2));
+    }
+  };
+
   private AlignmentI lastmatchedAl = null;
 
   private SequenceIdMatcher matcher = null;
@@ -517,156 +533,128 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
           Map<String, FeatureColourI> visible, boolean visOnly,
           boolean nonpos)
   {
-    StringBuilder out = new StringBuilder(256);
-    boolean featuresGen = false;
-    if (visOnly && !nonpos && (visible == null || visible.size() < 1))
+    if (visOnly && !nonpos && (visible == null || visible.isEmpty()))
     {
       // no point continuing.
       return "No Features Visible";
     }
 
-    if (visible != null && visOnly)
+    /*
+     * write out feature colours (if we know them)
+     */
+    // TODO: decide if feature links should also be written here ?
+    StringBuilder out = new StringBuilder(256);
+    if (visible != null && visOnly) // todo why visOnly test?
     {
-      // write feature colours only if we're given them and we are generating
-      // viewed features
-      // TODO: decide if feature links should also be written here ?
-      Iterator<String> en = visible.keySet().iterator();
-      while (en.hasNext())
+      for (Entry<String, FeatureColourI> featureColour : visible.entrySet())
       {
-        String featureType = en.next().toString();
-        FeatureColourI colour = visible.get(featureType);
-        out.append(colour.toJalviewFormat(featureType)).append(newline);
+        FeatureColourI colour = featureColour.getValue();
+        out.append(colour.toJalviewFormat(featureColour.getKey())).append(
+                newline);
       }
     }
 
     // Work out which groups are both present and visible
-    List<String> groups = new ArrayList<String>();
-    int groupIndex = 0;
-    boolean isnonpos = false;
+    Set<String> groups = new HashSet<String>();
+    String[] types = visible == null ? null : visible.keySet().toArray(
+            new String[visible.keySet().size()]);
 
-    SequenceFeature[] features;
     for (int i = 0; i < sequences.length; i++)
     {
-      features = sequences[i].getSequenceFeatures();
-      if (features != null)
+      groups.addAll(sequences[i].getFeatures()
+              .getFeatureGroups(true, types));
+      if (nonpos)
       {
-        for (int j = 0; j < features.length; j++)
-        {
-          isnonpos = features[j].begin == 0 && features[j].end == 0;
-          if ((!nonpos && isnonpos)
-                  || (!isnonpos && visOnly && !visible
-                          .containsKey(features[j].type)))
-          {
-            continue;
-          }
-
-          if (features[j].featureGroup != null
-                  && !groups.contains(features[j].featureGroup))
-          {
-            groups.add(features[j].featureGroup);
-          }
-        }
+        groups.addAll(sequences[i].getFeatures().getFeatureGroups(false,
+                types));
       }
     }
 
-    String group = null;
-    do
+    /*
+     * sort distinct groups so null group is output last
+     */
+    List<String> sortedGroups = new ArrayList<String>(groups);
+    Collections.sort(sortedGroups, SORT_NULL_LAST);
+
+    // TODO check where null group should be output
+    boolean foundSome = false;
+    for (String group : sortedGroups)
     {
-      if (groups.size() > 0 && groupIndex < groups.size())
+      if (group != null)
       {
-        group = groups.get(groupIndex);
         out.append(newline);
         out.append("STARTGROUP").append(TAB);
         out.append(group);
         out.append(newline);
       }
-      else
-      {
-        group = null;
-      }
 
+      /*
+       * output features within groups (non-positional first if wanted)
+       */
       for (int i = 0; i < sequences.length; i++)
       {
-        features = sequences[i].getSequenceFeatures();
-        if (features != null)
+        List<SequenceFeature> features = new ArrayList<SequenceFeature>();
+        if (nonpos)
         {
-          for (SequenceFeature sequenceFeature : features)
-          {
-            isnonpos = sequenceFeature.begin == 0
-                    && sequenceFeature.end == 0;
-            if ((!nonpos && isnonpos)
-                    || (!isnonpos && visOnly && !visible
-                            .containsKey(sequenceFeature.type)))
-            {
-              // skip if feature is nonpos and we ignore them or if we only
-              // output visible and it isn't non-pos and it's not visible
-              continue;
-            }
+          features.addAll(sequences[i].getFeatures().getFeaturesForGroup(
+                  false, group, types));
+        }
+        features.addAll(sequences[i].getFeatures().getFeaturesForGroup(
+                true, group, types));
 
-            if (group != null
-                    && (sequenceFeature.featureGroup == null || !sequenceFeature.featureGroup
-                            .equals(group)))
+        for (SequenceFeature sequenceFeature : features)
+        {
+          // we have features to output
+          foundSome = true;
+          if (sequenceFeature.description == null
+                  || sequenceFeature.description.equals(""))
+          {
+            out.append(sequenceFeature.type).append(TAB);
+          }
+          else
+          {
+            if (sequenceFeature.links != null
+                    && sequenceFeature.getDescription().indexOf("<html>") == -1)
             {
-              continue;
+              out.append("<html>");
             }
 
-            if (group == null && sequenceFeature.featureGroup != null)
-            {
-              continue;
-            }
-            // we have features to output
-            featuresGen = true;
-            if (sequenceFeature.description == null
-                    || sequenceFeature.description.equals(""))
-            {
-              out.append(sequenceFeature.type).append(TAB);
-            }
-            else
+            out.append(sequenceFeature.description);
+            if (sequenceFeature.links != null)
             {
-              if (sequenceFeature.links != null
-                      && sequenceFeature.getDescription().indexOf("<html>") == -1)
-              {
-                out.append("<html>");
-              }
-
-              out.append(sequenceFeature.description);
-              if (sequenceFeature.links != null)
+              for (int l = 0; l < sequenceFeature.links.size(); l++)
               {
-                for (int l = 0; l < sequenceFeature.links.size(); l++)
-                {
-                  String label = sequenceFeature.links.elementAt(l);
-                  String href = label.substring(label.indexOf("|") + 1);
-                  label = label.substring(0, label.indexOf("|"));
-
-                  if (sequenceFeature.description.indexOf(href) == -1)
-                  {
-                    out.append(" <a href=\"" + href + "\">" + label
-                            + "</a>");
-                  }
-                }
+                String label = sequenceFeature.links.elementAt(l);
+                String href = label.substring(label.indexOf("|") + 1);
+                label = label.substring(0, label.indexOf("|"));
 
-                if (sequenceFeature.getDescription().indexOf("</html>") == -1)
+                if (sequenceFeature.description.indexOf(href) == -1)
                 {
-                  out.append("</html>");
+                  out.append(" <a href=\"" + href + "\">" + label + "</a>");
                 }
               }
 
-              out.append(TAB);
+              if (sequenceFeature.getDescription().indexOf("</html>") == -1)
+              {
+                out.append("</html>");
+              }
             }
-            out.append(sequences[i].getName());
-            out.append("\t-1\t");
-            out.append(sequenceFeature.begin);
+
             out.append(TAB);
-            out.append(sequenceFeature.end);
+          }
+          out.append(sequences[i].getName());
+          out.append("\t-1\t");
+          out.append(sequenceFeature.begin);
+          out.append(TAB);
+          out.append(sequenceFeature.end);
+          out.append(TAB);
+          out.append(sequenceFeature.type);
+          if (!Float.isNaN(sequenceFeature.score))
+          {
             out.append(TAB);
-            out.append(sequenceFeature.type);
-            if (!Float.isNaN(sequenceFeature.score))
-            {
-              out.append(TAB);
-              out.append(sequenceFeature.score);
-            }
-            out.append(newline);
+            out.append(sequenceFeature.score);
           }
+          out.append(newline);
         }
       }
 
@@ -675,21 +663,10 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI
         out.append("ENDGROUP").append(TAB);
         out.append(group);
         out.append(newline);
-        groupIndex++;
       }
-      else
-      {
-        break;
-      }
-
-    } while (groupIndex < groups.size() + 1);
-
-    if (!featuresGen)
-    {
-      return "No Features Visible";
     }
 
-    return out.toString();
+    return foundSome ? out.toString() : "No Features Visible";
   }
 
   /**