From fefc840cd97faf34b69ca387e8a38e3b9c79b92f Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Wed, 17 Aug 2016 11:06:04 +0100
Subject: [PATCH] JAL-2172 handle case where column selection is not ordered

---
 src/jalview/analysis/Grouping.java      |    8 +++++++-
 test/jalview/analysis/GroupingTest.java |   26 +++++++++++++++-----------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/jalview/analysis/Grouping.java b/src/jalview/analysis/Grouping.java
index 51a818f..ddf483b 100644
--- a/src/jalview/analysis/Grouping.java
+++ b/src/jalview/analysis/Grouping.java
@@ -25,6 +25,7 @@ import jalview.datamodel.SequenceGroup;
 import jalview.datamodel.SequenceI;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -134,7 +135,12 @@ public class Grouping
     {
       spos[i++] = pos.intValue();
     }
-    ;
+
+    /*
+     * ensure column selection is in ascending order
+     */
+    Arrays.sort(spos);
+
     for (i = 0; i < sequences.length; i++)
     {
       int slen = sequences[i].getLength();
diff --git a/test/jalview/analysis/GroupingTest.java b/test/jalview/analysis/GroupingTest.java
index 55823e4..3afa6e5 100644
--- a/test/jalview/analysis/GroupingTest.java
+++ b/test/jalview/analysis/GroupingTest.java
@@ -27,7 +27,6 @@ import jalview.datamodel.Sequence;
 import jalview.datamodel.SequenceGroup;
 import jalview.datamodel.SequenceI;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 
 import org.testng.AssertJUnit;
@@ -45,21 +44,21 @@ public class GroupingTest
 
   Sequence s5 = new Sequence("s5", "AAAADDEDTTEE");
 
-  SequenceGroup sg1 = new SequenceGroup(Arrays.asList(new SequenceI[] { s1,
+  SequenceGroup sg_12 = new SequenceGroup(Arrays.asList(new SequenceI[] { s1,
       s2 }), "Group1", null, false, false, false, 0, 5);
 
-  SequenceGroup sg2 = new SequenceGroup(Arrays.asList(new SequenceI[] { s3,
+  SequenceGroup sg_345 = new SequenceGroup(Arrays.asList(new SequenceI[] { s3,
       s4, s5 }), "Group2", null, false, false, false, 0, 5);
 
   AlignmentI alignment = new Alignment(
           new SequenceI[] { s1, s2, s3, s4, s5 });
 
-  int[] positions = new int[] { 1, 7, 9 };
-
   @Test(groups = { "Functional" })
   public void testMakeGroupsWithBoth()
   {
-    ArrayList<String> str = new ArrayList<String>();
+    int[] positions = new int[] { 1, 7, 9 };
+    String[] str = new String[alignment.getHeight()];
+    int seq = 0;
     for (SequenceI s : alignment.getSequences())
     {
       StringBuilder sb = new StringBuilder();
@@ -67,12 +66,17 @@ public class GroupingTest
       {
         sb.append(s.getCharAt(p));
       }
-      str.add(sb.toString());
+      str[seq++] = sb.toString();
     }
     SequenceGroup[] seqgroupsString = Grouping.makeGroupsFrom(
-            alignment.getSequencesArray(),
-            str.toArray(new String[str.size()]),
-            Arrays.asList(new SequenceGroup[] { sg1, sg2 }));
+            alignment.getSequencesArray(), str,
+            Arrays.asList(new SequenceGroup[] { sg_12, sg_345 }));
+
+    /*
+     * test for the case where column selections are not added in
+     * left to right order
+     */
+    positions = new int[] { 7, 9, 1 };
     ColumnSelection cs = new ColumnSelection();
     for (int p : positions)
     {
@@ -80,7 +84,7 @@ public class GroupingTest
     }
     SequenceGroup[] seqgroupsColSel = Grouping.makeGroupsFromCols(
             alignment.getSequencesArray(), cs,
-            Arrays.asList(new SequenceGroup[] { sg1, sg2 }));
+            Arrays.asList(new SequenceGroup[] { sg_12, sg_345 }));
     AssertJUnit
             .assertEquals(seqgroupsString.length, seqgroupsColSel.length);
     for (int p = 0; p < seqgroupsString.length; p++)
-- 
1.7.10.2