JAL-1152 bug fix for 'no sort' case + tests for autocalc placement

diff --git a/src/jalview/analysis/AnnotationSorter.java b/src/jalview/analysis/AnnotationSorter.java
index fa9c1a8..b9e9ed5 100644 (file)
--- a/src/jalview/analysis/AnnotationSorter.java
+++ b/src/jalview/analysis/AnnotationSorter.java
@@ -21,7 +21,7 @@ public class AnnotationSorter
   {
     SEQUENCE_AND_LABEL, LABEL_AND_SEQUENCE, NONE
   }
-  
+
   private final AlignmentI alignment;
 
   private boolean showAutocalcAbove;
@@ -158,7 +158,7 @@ public class AnnotationSorter
       return 0;
     }
   };
-  
+
   /**
    * Sort by the specified ordering of sequence-specific annotations.
    * 
@@ -168,16 +168,13 @@ public class AnnotationSorter
   public void sort(AlignmentAnnotation[] alignmentAnnotations,
           SequenceAnnotationOrder order)
   {
-    if (order != SequenceAnnotationOrder.NONE)
-    {
-      Comparator<? super AlignmentAnnotation> comparator = getComparator(order);
+    Comparator<? super AlignmentAnnotation> comparator = getComparator(order);
 
-      if (alignmentAnnotations != null)
+    if (alignmentAnnotations != null)
+    {
+      synchronized (alignmentAnnotations)
       {
-        synchronized (alignmentAnnotations)
-        {
-          Arrays.sort(alignmentAnnotations, comparator);
-        }
+        Arrays.sort(alignmentAnnotations, comparator);
       }
     }
   }

diff --git a/src/jalview/gui/AlignViewport.java b/src/jalview/gui/AlignViewport.java
index 5c07f6e..d331e82 100644 (file)
--- a/src/jalview/gui/AlignViewport.java
+++ b/src/jalview/gui/AlignViewport.java
@@ -366,7 +366,7 @@ public class AlignViewport extends AlignmentViewport implements
     sortByTree = Cache.getDefault("SORT_BY_TREE", false);
     followSelection = Cache.getDefault("FOLLOW_SELECTIONS", true);
     sortAnnotationsBy = SequenceAnnotationOrder.valueOf(Cache.getDefault(
-            "SORT_ANNOTATIONS", SequenceAnnotationOrder.SEQUENCE_AND_LABEL.name()));
+            "SORT_ANNOTATIONS", SequenceAnnotationOrder.NONE.name()));
     showAutocalculatedAbove = Cache
             .getDefault("SHOW_AUTOCALC_ABOVE", false);
   }

diff --git a/test/jalview/analysis/AnnotationSorterTest.java b/test/jalview/analysis/AnnotationSorterTest.java
index 55dcf6d..ba2162d 100644 (file)
--- a/test/jalview/analysis/AnnotationSorterTest.java
+++ b/test/jalview/analysis/AnnotationSorterTest.java
@@ -85,7 +85,7 @@ public class AnnotationSorterTest
    * </ul>
    */
   @Test
-  public void testSortBySequenceAndType()
+  public void testSortBySequenceAndType_autocalcLast()
   {
     // @formatter:off
     anns[0].sequenceRef = al.getSequenceAt(1); anns[0].label = "label0";
@@ -109,6 +109,33 @@ public class AnnotationSorterTest
   }
 
   /**
+   * Variant with autocalculated annotations sorting to front
+   */
+  @Test
+  public void testSortBySequenceAndType_autocalcFirst()
+  {
+    // @formatter:off
+    anns[0].sequenceRef = al.getSequenceAt(1); anns[0].label = "label0";
+    anns[1].sequenceRef = al.getSequenceAt(3); anns[1].label = "structure";
+    anns[2].sequenceRef = al.getSequenceAt(3); anns[2].label = "iron";
+    anns[3].sequenceRef = null;                anns[3].label = "Quality";
+    anns[4].sequenceRef = null;                anns[4].label = "Consensus";
+    anns[5].sequenceRef = al.getSequenceAt(0); anns[5].label = "label5";
+    anns[6].sequenceRef = al.getSequenceAt(3); anns[6].label = "IRP";
+    // @formatter:on
+
+    AnnotationSorter testee = new AnnotationSorter(al, true);
+    testee.sort(anns, SequenceAnnotationOrder.SEQUENCE_AND_LABEL);
+    assertEquals("Quality", anns[0].label); // non-sequence annotations
+    assertEquals("Consensus", anns[1].label); // retain ordering
+    assertEquals("label5", anns[2].label); // for sequence 0
+    assertEquals("label0", anns[3].label); // for sequence 1
+    assertEquals("iron", anns[4].label); // sequence 3 /iron
+    assertEquals("IRP", anns[5].label); // sequence 3/IRP
+    assertEquals("structure", anns[6].label); // sequence 3/structure
+  }
+
+  /**
    * Test sorting by annotation type (label) within sequence order, including
    * <ul>
    * <li>annotations with no sequence reference - sort to end keeping mutual
@@ -121,7 +148,7 @@ public class AnnotationSorterTest
    * </ul>
    */
   @Test
-  public void testSortByTypeAndSequence()
+  public void testSortByTypeAndSequence_autocalcLast()
   {
     // @formatter:off
     anns[0].sequenceRef = al.getSequenceAt(1); anns[0].label = "label0";
@@ -144,6 +171,61 @@ public class AnnotationSorterTest
     assertEquals("Consensus", anns[6].label); // retain ordering
   }
 
+  /**
+   * Variant of test with autocalculated annotations sorted to front
+   */
+  @Test
+  public void testSortByTypeAndSequence_autocalcFirst()
+  {
+    // @formatter:off
+    anns[0].sequenceRef = al.getSequenceAt(1); anns[0].label = "label0";
+    anns[1].sequenceRef = al.getSequenceAt(3); anns[1].label = "structure";
+    anns[2].sequenceRef = al.getSequenceAt(3); anns[2].label = "iron";
+    anns[3].sequenceRef = null;                anns[3].label = "Quality";
+    anns[4].sequenceRef = null;                anns[4].label = "Consensus";
+    anns[5].sequenceRef = al.getSequenceAt(0); anns[5].label = "IRON";
+    anns[6].sequenceRef = al.getSequenceAt(2); anns[6].label = "Structure";
+    // @formatter:on
+
+    AnnotationSorter testee = new AnnotationSorter(al, true);
+    testee.sort(anns, SequenceAnnotationOrder.LABEL_AND_SEQUENCE);
+    assertEquals("Quality", anns[0].label); // non-sequence annotations
+    assertEquals("Consensus", anns[1].label); // retain ordering
+    assertEquals("IRON", anns[2].label); // IRON / sequence 0
+    assertEquals("iron", anns[3].label); // iron / sequence 3
+    assertEquals("label0", anns[4].label); // label0 / sequence 1
+    assertEquals("Structure", anns[5].label); // Structure / sequence 2
+    assertEquals("structure", anns[6].label); // structure / sequence 3
+  }
+
+  /**
+   * Variant of test with autocalculated annotations sorted to front but
+   * otherwise no change.
+   */
+  @Test
+  public void testNoSort_autocalcFirst()
+  {
+    // @formatter:off
+    anns[0].sequenceRef = al.getSequenceAt(1); anns[0].label = "label0";
+    anns[1].sequenceRef = al.getSequenceAt(3); anns[1].label = "structure";
+    anns[2].sequenceRef = al.getSequenceAt(3); anns[2].label = "iron";
+    anns[3].sequenceRef = null;                anns[3].label = "Quality";
+    anns[4].sequenceRef = null;                anns[4].label = "Consensus";
+    anns[5].sequenceRef = al.getSequenceAt(0); anns[5].label = "IRON";
+    anns[6].sequenceRef = al.getSequenceAt(2); anns[6].label = "Structure";
+    // @formatter:on
+
+    AnnotationSorter testee = new AnnotationSorter(al, true);
+    testee.sort(anns, SequenceAnnotationOrder.NONE);
+    assertEquals("Quality", anns[0].label); // non-sequence annotations
+    assertEquals("Consensus", anns[1].label); // retain ordering
+    assertEquals("label0", anns[2].label);
+    assertEquals("structure", anns[3].label);
+    assertEquals("iron", anns[4].label);
+    assertEquals("IRON", anns[5].label);
+    assertEquals("Structure", anns[6].label);
+  }
+
   @Test
   public void testSort_timingPresorted()
   {