JAL-2360 JAL-2371 updates from review CR-JAL-1

diff --git a/src/jalview/datamodel/SequenceGroup.java b/src/jalview/datamodel/SequenceGroup.java
index 8218ba7..c77569e 100755 (executable)
--- a/src/jalview/datamodel/SequenceGroup.java
+++ b/src/jalview/datamodel/SequenceGroup.java
@@ -1394,9 +1394,8 @@ public class SequenceGroup implements AnnotatedCollectionI
   @Override
   public boolean isNucleotide()
   {
-    if (context != null && context instanceof AlignmentI)
-    {
-      return ((AlignmentI) context).isNucleotide();
+    if (context != null) {
+      return context.isNucleotide();
     }
     return false;
   }

diff --git a/src/jalview/schemes/CollectionColourScheme.java b/src/jalview/schemes/CollectionColourScheme.java
index a212795..ebff6e6 100644 (file)
--- a/src/jalview/schemes/CollectionColourScheme.java
+++ b/src/jalview/schemes/CollectionColourScheme.java
@@ -13,13 +13,12 @@ import java.awt.Color;
 import java.util.Map;
 
 /**
- * A data bean that holds the information to determine the colour scheme of an
- * alignment (or subgroup), consisting of
+ * A class that computes the colouring of an alignment (or subgroup). Currently
+ * the factors that may influence residue colouring are
  * <ul>
- * <li>the colour scheme that determines the colour of a residue</li>
+ * <li>the colour scheme that provides a colour for each aligned residue</li>
  * <li>any threshold for colour, based on percentage identity with consensus</li>
- * <li>any graduation based on consensus</li>
- * <li>the consensus data</li>
+ * <li>any graduation based on conservation of physico-chemical properties</li>
  * </ul>
  * 
  * @author gmcarstairs

diff --git a/src/jalview/schemes/ColourSchemeProperty.java b/src/jalview/schemes/ColourSchemeProperty.java
index 5190c72..3e8e87a 100755 (executable)
--- a/src/jalview/schemes/ColourSchemeProperty.java
+++ b/src/jalview/schemes/ColourSchemeProperty.java
@@ -44,6 +44,8 @@ public class ColourSchemeProperty
    * Returns a colour scheme for the given name, with which the given data may
    * be coloured. The name is not case-sensitive, and may be one of
    * <ul>
+   * <li>any currently registered colour scheme; Jalview by default provides</li>
+   * <ul>
    * <li>Clustal</li>
    * <li>Blosum62</li>
    * <li>% Identity</li>
@@ -58,11 +60,11 @@ public class ColourSchemeProperty
    * <li>Purine/Pyrimidine</li>
    * <li>T-Coffee Scores</li>
    * <li>RNA Helices</li>
-   * <li>User Defined</li>
-   * <li>an AWT colour name e.g. red</li>
-   * <li>an AWT hex rgb colour e.g. ff2288</li>
-   * <li>residue colours list e.g. D,E=red;K,R,H=0022FF;c=yellow</li>
    * </ul>
+   * <li>the name of a programmatically added colour scheme</li> <li>an AWT
+   * colour name e.g. red</li> <li>an AWT hex rgb colour e.g. ff2288</li> <li>
+   * residue colours list e.g. D,E=red;K,R,H=0022FF;c=yellow</li> </ul>
+   * 
    * If none of these formats is matched, the string is converted to a colour
    * using a hashing algorithm. For name "None", returns null.
    * 

diff --git a/src/jalview/viewmodel/AlignmentViewport.java b/src/jalview/viewmodel/AlignmentViewport.java
index a9d3b77..69879b7 100644 (file)
--- a/src/jalview/viewmodel/AlignmentViewport.java
+++ b/src/jalview/viewmodel/AlignmentViewport.java
@@ -613,7 +613,7 @@ public abstract class AlignmentViewport implements AlignViewportI,
     // via changecolour
 
     /*
-     * only instantiate colour scheme once, thereafter update it
+     * only instantiate collection colour scheme once, thereafter update it
      * this means that any conservation or PID threshold settings
      * persist when the alignment colour scheme is changed
      */

diff --git a/test/jalview/io/AnnotationFileIOTest.java b/test/jalview/io/AnnotationFileIOTest.java
index 5b90e19..885c673 100644 (file)
--- a/test/jalview/io/AnnotationFileIOTest.java
+++ b/test/jalview/io/AnnotationFileIOTest.java
@@ -70,7 +70,7 @@ public class AnnotationFileIOTest
     }
   }
 
-  public static AlignmentI readAlignmentFile(File f)
+  AlignmentI readAlignmentFile(File f)
   {
     System.out.println("Reading file: " + f);
     String ff = f.getPath();
@@ -106,9 +106,7 @@ public class AnnotationFileIOTest
    *          - label for IO class used to write and read back in the data from
    *          f
    */
-
-  // @Test(groups ={ "Functional" })
-  public static void testAnnotationFileIO(String testname, File f,
+  void testAnnotationFileIO(String testname, File f,
           File annotFile)
   {
     System.out.println("Test: " + testname + "\nReading annotation file '"

diff --git a/test/jalview/schemes/ColourSchemePropertyTest.java b/test/jalview/schemes/ColourSchemePropertyTest.java
index c1c6846..11562b8 100644 (file)
--- a/test/jalview/schemes/ColourSchemePropertyTest.java
+++ b/test/jalview/schemes/ColourSchemePropertyTest.java
@@ -76,46 +76,60 @@ public class ColourSchemePropertyTest
     SequenceI seq = new Sequence("Seq1", "abcd");
     AlignmentI al = new Alignment(new SequenceI[] { seq });
     // the strings here correspond to JalviewColourScheme.toString() values
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "Clustal") instanceof ClustalxColourScheme);
+    ColourSchemeI cs = ColourSchemeProperty.getColourScheme(al, "Clustal");
+    assertTrue(cs instanceof ClustalxColourScheme);
     // not case-sensitive
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "CLUSTAL") instanceof ClustalxColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "clustal") instanceof ClustalxColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "Blosum62") instanceof Blosum62ColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "% Identity") instanceof PIDColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "Zappo") instanceof ZappoColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "Taylor") instanceof TaylorColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "Hydrophobic") instanceof HydrophobicColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "Helix Propensity") instanceof HelixColourScheme);
-    assertTrue(ColourSchemeProperty
-            .getColourScheme(al, "Strand Propensity") instanceof StrandColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "Turn Propensity") instanceof TurnColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "Buried Index") instanceof BuriedColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "Nucleotide") instanceof NucleotideColourScheme);
-    assertTrue(ColourSchemeProperty
-            .getColourScheme(al, "Purine/Pyrimidine") instanceof PurinePyrimidineColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "T-Coffee Scores") instanceof TCoffeeColourScheme);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "RNA Helices") instanceof RNAHelicesColour);
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "User Defined") instanceof UserColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "CLUSTAL");
+    assertTrue(cs instanceof ClustalxColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "clustal");
+    assertTrue(cs instanceof ClustalxColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "Blosum62");
+    assertTrue(cs instanceof Blosum62ColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "% Identity");
+    assertTrue(cs instanceof PIDColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "Zappo");
+    assertTrue(cs instanceof ZappoColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "Taylor");
+    assertTrue(cs instanceof TaylorColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "Hydrophobic");
+    assertTrue(cs instanceof HydrophobicColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "Helix Propensity");
+    assertTrue(cs instanceof HelixColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "Strand Propensity");
+    assertTrue(cs instanceof StrandColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "Turn Propensity");
+    assertTrue(cs instanceof TurnColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "Buried Index");
+    assertTrue(cs instanceof BuriedColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "Nucleotide");
+    assertTrue(cs instanceof NucleotideColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "Purine/Pyrimidine");
+    assertTrue(cs instanceof PurinePyrimidineColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "T-Coffee Scores");
+    assertTrue(cs instanceof TCoffeeColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "RNA Helices");
+    assertTrue(cs instanceof RNAHelicesColour);
     // 'None' is a special value
     assertNull(ColourSchemeProperty.getColourScheme(al, "None"));
     assertNull(ColourSchemeProperty.getColourScheme(al, "none"));
     // default is to convert the name into a fixed colour
-    assertTrue(ColourSchemeProperty.getColourScheme(al, "elephants") instanceof UserColourScheme);
+    cs = ColourSchemeProperty.getColourScheme(al, "elephants");
+    assertTrue(cs instanceof UserColourScheme);
 
     /*
      * explicit aa colours
      */
-    UserColourScheme cs = (UserColourScheme) ColourSchemeProperty
+    UserColourScheme ucs = (UserColourScheme) ColourSchemeProperty
             .getColourScheme(al,
             "R,G=red;C=blue;c=green;Q=10,20,30;S,T=11ffdd");
-    assertEquals(cs.findColour('H'), Color.white);
-    assertEquals(cs.findColour('R'), Color.red);
-    assertEquals(cs.findColour('r'), Color.red);
-    assertEquals(cs.findColour('G'), Color.red);
-    assertEquals(cs.findColour('C'), Color.blue);
-    assertEquals(cs.findColour('c'), Color.green);
-    assertEquals(cs.findColour('Q'), new Color(10, 20, 30));
-    assertEquals(cs.findColour('S'), new Color(0x11ffdd));
-    assertEquals(cs.findColour('T'), new Color(0x11ffdd));
+    assertEquals(ucs.findColour('H'), Color.white);
+    assertEquals(ucs.findColour('R'), Color.red);
+    assertEquals(ucs.findColour('r'), Color.red);
+    assertEquals(ucs.findColour('G'), Color.red);
+    assertEquals(ucs.findColour('C'), Color.blue);
+    assertEquals(ucs.findColour('c'), Color.green);
+    assertEquals(ucs.findColour('Q'), new Color(10, 20, 30));
+    assertEquals(ucs.findColour('S'), new Color(0x11ffdd));
+    assertEquals(ucs.findColour('T'), new Color(0x11ffdd));
   }
 }

diff --git a/test/jalview/schemes/ColourSchemesTest.java b/test/jalview/schemes/ColourSchemesTest.java
index 39d58f8..4618ed7 100644 (file)
--- a/test/jalview/schemes/ColourSchemesTest.java
+++ b/test/jalview/schemes/ColourSchemesTest.java
@@ -2,16 +2,15 @@ package jalview.schemes;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertSame;
 import static org.testng.Assert.assertTrue;
 
 import jalview.bin.Cache;
 import jalview.bin.Jalview;
-import jalview.datamodel.AlignmentI;
 import jalview.datamodel.AnnotatedCollectionI;
 import jalview.datamodel.SequenceCollectionI;
 import jalview.datamodel.SequenceI;
 import jalview.gui.AlignFrame;
-import jalview.gui.AlignViewport;
 import jalview.gui.Desktop;
 import jalview.gui.SequenceRenderer;
 import jalview.io.DataSourceType;
@@ -195,77 +194,19 @@ public class ColourSchemesTest
   }
 
   @Test(groups = "Functional")
-  public void testGetColourScheme_forViewport()
+  public void testGetColourScheme()
   {
     AlignFrame af = new FileLoader().LoadFileWaitTillLoaded(
             ">seq1\nAGLRTWQU", DataSourceType.PASTE);
-    ColourSchemes cs = ColourSchemes.getInstance();
-
-    AlignViewport viewport = af.getViewport();
-    AlignmentI alignment = viewport.getAlignment();
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Blosum62.toString(),
-            alignment) instanceof Blosum62ColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Buried.toString(),
-            alignment) instanceof BuriedColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Clustal.toString(),
-            alignment) instanceof ClustalxColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Helix.toString(),
-            alignment) instanceof HelixColourScheme);
-    assertTrue(cs.getColourScheme(
-            JalviewColourScheme.Hydrophobic.toString(), alignment) instanceof HydrophobicColourScheme);
-    assertTrue(cs.getColourScheme(
-            JalviewColourScheme.Nucleotide.toString(), alignment) instanceof NucleotideColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.PID.toString(),
-            alignment) instanceof PIDColourScheme);
-    assertTrue(cs.getColourScheme(
-            JalviewColourScheme.PurinePyrimidine.toString(), alignment) instanceof PurinePyrimidineColourScheme);
-    assertTrue(cs.getColourScheme(
-            JalviewColourScheme.RNAHelices.toString(), alignment) instanceof RNAHelicesColour);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Strand.toString(),
-            alignment) instanceof StrandColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Taylor.toString(),
-            alignment) instanceof TaylorColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.TCoffee.toString(),
-            alignment) instanceof TCoffeeColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Turn.toString(),
-            alignment) instanceof TurnColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Zappo.toString(),
-            alignment) instanceof ZappoColourScheme);
-    af.closeMenuItem_actionPerformed(true);
-  }
-
-  @Test(groups = "Functional")
-  public void testGetColourScheme_forAnnotatedCollection()
-  {
-    AlignFrame af = new FileLoader().LoadFileWaitTillLoaded(
-            ">seq1\nAGLRTWQU", DataSourceType.PASTE);
-    ColourSchemes cs = ColourSchemes.getInstance();
+    ColourSchemes schemes = ColourSchemes.getInstance();
 
     AnnotatedCollectionI al = af.getViewport().getAlignment();
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Blosum62.toString(),
-            al) instanceof Blosum62ColourScheme);
-    assertTrue(cs
-            .getColourScheme(JalviewColourScheme.Buried.toString(), al) instanceof BuriedColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Clustal.toString(),
-            al) instanceof ClustalxColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Helix.toString(), al) instanceof HelixColourScheme);
-    assertTrue(cs.getColourScheme(
-            JalviewColourScheme.Hydrophobic.toString(), al) instanceof HydrophobicColourScheme);
-    assertTrue(cs.getColourScheme(
-            JalviewColourScheme.Nucleotide.toString(), al) instanceof NucleotideColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.PID.toString(), al) instanceof PIDColourScheme);
-    assertTrue(cs.getColourScheme(
-            JalviewColourScheme.PurinePyrimidine.toString(), al) instanceof PurinePyrimidineColourScheme);
-    assertTrue(cs.getColourScheme(
-            JalviewColourScheme.RNAHelices.toString(), al) instanceof RNAHelicesColour);
-    assertTrue(cs
-            .getColourScheme(JalviewColourScheme.Strand.toString(), al) instanceof StrandColourScheme);
-    assertTrue(cs
-            .getColourScheme(JalviewColourScheme.Taylor.toString(), al) instanceof TaylorColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.TCoffee.toString(),
-            al) instanceof TCoffeeColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Turn.toString(), al) instanceof TurnColourScheme);
-    assertTrue(cs.getColourScheme(JalviewColourScheme.Zappo.toString(), al) instanceof ZappoColourScheme);
+
+    for (JalviewColourScheme cs : JalviewColourScheme.values())
+    {
+      ColourSchemeI registered = schemes.getColourScheme(cs.toString(), al);
+      assertSame(registered.getClass(), cs.getSchemeClass());
+    }
     af.closeMenuItem_actionPerformed(true);
   }
   

diff --git a/test/jalview/util/ColorUtilsTest.java b/test/jalview/util/ColorUtilsTest.java
index d267ebc..fa4091f 100644 (file)
--- a/test/jalview/util/ColorUtilsTest.java
+++ b/test/jalview/util/ColorUtilsTest.java
@@ -230,6 +230,8 @@ public class ColorUtilsTest
   {
     assertEquals(Color.white, ColorUtils.createColourFromName(null));
     assertEquals(new Color(20, 20, 20), ColorUtils.createColourFromName(""));
+    assertEquals(new Color(98, 131, 171),
+            ColorUtils.createColourFromName("None")); // no special treatment!
     assertEquals(new Color(123, 211, 122),
             ColorUtils.createColourFromName("hello world"));
     assertEquals(new Color(27, 147, 112),