From b0832234b2a89510d715951d94ae213e449ca6b2 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Fri, 27 Jan 2017 10:40:43 +0000 Subject: [PATCH] JAL-2360 JAL-2371 updates from review CR-JAL-1 --- src/jalview/datamodel/SequenceGroup.java | 5 +- src/jalview/schemes/CollectionColourScheme.java | 9 +-- src/jalview/schemes/ColourSchemeProperty.java | 10 ++- src/jalview/viewmodel/AlignmentViewport.java | 2 +- test/jalview/io/AnnotationFileIOTest.java | 6 +- test/jalview/schemes/ColourSchemePropertyTest.java | 74 +++++++++++-------- test/jalview/schemes/ColourSchemesTest.java | 77 +++----------------- test/jalview/util/ColorUtilsTest.java | 2 + 8 files changed, 70 insertions(+), 115 deletions(-) diff --git a/src/jalview/datamodel/SequenceGroup.java b/src/jalview/datamodel/SequenceGroup.java index 8218ba7..c77569e 100755 --- 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 --- 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 * * * @author gmcarstairs diff --git a/src/jalview/schemes/ColourSchemeProperty.java b/src/jalview/schemes/ColourSchemeProperty.java index 5190c72..3e8e87a 100755 --- 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 * + * * 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 --- 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 --- 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 --- 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 --- 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 --- 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), -- 1.7.10.2