From 3f91bb1385ab9ac8fdd15e3bc9378f5b554e1642 Mon Sep 17 00:00:00 2001 From: kiramt Date: Wed, 24 May 2017 10:44:47 +0100 Subject: [PATCH] JAL-2034 Added isDefined flag to SequenceGroup --- src/jalview/datamodel/Alignment.java | 24 +++++++------- src/jalview/datamodel/SequenceGroup.java | 44 +++++++++++++++++++++---- src/jalview/viewmodel/AlignmentViewport.java | 18 +++++----- test/jalview/datamodel/SequenceGroupTest.java | 25 ++++++++++++-- test/jalview/gui/AlignViewportTest.java | 23 +++++++++---- 5 files changed, 95 insertions(+), 39 deletions(-) diff --git a/src/jalview/datamodel/Alignment.java b/src/jalview/datamodel/Alignment.java index fcb6109..5d91b36 100755 --- a/src/jalview/datamodel/Alignment.java +++ b/src/jalview/datamodel/Alignment.java @@ -73,7 +73,7 @@ public class Alignment implements AlignmentI groups = Collections.synchronizedList(new ArrayList()); hiddenSequences = new HiddenSequences(this); hiddenCols = new HiddenColumns(); - codonFrameList = new ArrayList(); + codonFrameList = new ArrayList<>(); nucleotide = Comparison.isNucleotide(seqs); @@ -405,7 +405,7 @@ public class Alignment implements AlignmentI @Override public SequenceGroup[] findAllGroups(SequenceI s) { - ArrayList temp = new ArrayList(); + ArrayList temp = new ArrayList<>(); synchronized (groups) { @@ -456,7 +456,7 @@ public class Alignment implements AlignmentI return; } } - sg.setContext(this); + sg.setContext(this, true); groups.add(sg); } } @@ -533,7 +533,7 @@ public class Alignment implements AlignmentI } for (SequenceGroup sg : groups) { - sg.setContext(null); + sg.setContext(null, false); } groups.clear(); } @@ -549,7 +549,7 @@ public class Alignment implements AlignmentI { removeAnnotationForGroup(g); groups.remove(g); - g.setContext(null); + g.setContext(null, false); } } } @@ -1071,7 +1071,7 @@ public class Alignment implements AlignmentI { return; } - List toProcess = new ArrayList(); + List toProcess = new ArrayList<>(); toProcess.add(currentSeq); while (toProcess.size() > 0) { @@ -1124,7 +1124,7 @@ public class Alignment implements AlignmentI return; } // try to avoid using SequenceI.equals at this stage, it will be expensive - Set seqs = new LinkedIdentityHashSet(); + Set seqs = new LinkedIdentityHashSet<>(); for (int i = 0; i < getHeight(); i++) { @@ -1409,7 +1409,7 @@ public class Alignment implements AlignmentI { return null; } - List cframes = new ArrayList(); + List cframes = new ArrayList<>(); for (AlignedCodonFrame acf : getCodonFrames()) { if (acf.involvesSequence(seq)) @@ -1486,7 +1486,7 @@ public class Alignment implements AlignmentI if (sqs != null) { // avoid self append deadlock by - List toappendsq = new ArrayList(); + List toappendsq = new ArrayList<>(); synchronized (sqs) { for (SequenceI addedsq : sqs) @@ -1627,7 +1627,7 @@ public class Alignment implements AlignmentI @Override public Iterable findAnnotation(String calcId) { - List aa = new ArrayList(); + List aa = new ArrayList<>(); AlignmentAnnotation[] alignmentAnnotation = getAlignmentAnnotation(); if (alignmentAnnotation != null) { @@ -1648,7 +1648,7 @@ public class Alignment implements AlignmentI public Iterable findAnnotations(SequenceI seq, String calcId, String label) { - ArrayList aa = new ArrayList(); + ArrayList aa = new ArrayList<>(); for (AlignmentAnnotation ann : getAlignmentAnnotation()) { if ((calcId == null || (ann.getCalcId() != null && ann.getCalcId() @@ -1853,7 +1853,7 @@ public class Alignment implements AlignmentI @Override public Set getSequenceNames() { - Set names = new HashSet(); + Set names = new HashSet<>(); for (SequenceI seq : getSequences()) { names.add(seq.getName()); diff --git a/src/jalview/datamodel/SequenceGroup.java b/src/jalview/datamodel/SequenceGroup.java index 76ad093..8c29ca5 100755 --- a/src/jalview/datamodel/SequenceGroup.java +++ b/src/jalview/datamodel/SequenceGroup.java @@ -52,6 +52,12 @@ public class SequenceGroup implements AnnotatedCollectionI boolean colourText = false; /** + * True if the group is defined as a group on the alignment, false if it is + * just a selection. + */ + boolean isDefined = false; + + /** * after Olivier's non-conserved only character display */ boolean showNonconserved = false; @@ -59,7 +65,7 @@ public class SequenceGroup implements AnnotatedCollectionI /** * group members */ - private List sequences = new ArrayList(); + private List sequences = new ArrayList<>(); /** * representative sequence for this group (if any) @@ -161,7 +167,7 @@ public class SequenceGroup implements AnnotatedCollectionI this(); if (seqsel != null) { - sequences = new ArrayList(); + sequences = new ArrayList<>(); sequences.addAll(seqsel.sequences); if (seqsel.groupName != null) { @@ -311,7 +317,7 @@ public class SequenceGroup implements AnnotatedCollectionI } else { - List allSequences = new ArrayList(); + List allSequences = new ArrayList<>(); for (SequenceI seq : sequences) { allSequences.add(seq); @@ -1015,7 +1021,7 @@ public class SequenceGroup implements AnnotatedCollectionI { SequenceGroup sgroup = new SequenceGroup(this); SequenceI[] insect = getSequencesInOrder(alignment); - sgroup.sequences = new ArrayList(); + sgroup.sequences = new ArrayList<>(); for (int s = 0; insect != null && s < insect.length; s++) { if (map == null || map.containsKey(insect[s])) @@ -1242,7 +1248,7 @@ public class SequenceGroup implements AnnotatedCollectionI { // TODO add in other methods like 'getAlignmentAnnotation(String label), // etc' - ArrayList annot = new ArrayList(); + ArrayList annot = new ArrayList<>(); synchronized (sequences) { for (SequenceI seq : sequences) @@ -1274,7 +1280,7 @@ public class SequenceGroup implements AnnotatedCollectionI @Override public Iterable findAnnotation(String calcId) { - List aa = new ArrayList(); + List aa = new ArrayList<>(); if (calcId == null) { return aa; @@ -1293,7 +1299,7 @@ public class SequenceGroup implements AnnotatedCollectionI public Iterable findAnnotations(SequenceI seq, String calcId, String label) { - ArrayList aa = new ArrayList(); + ArrayList aa = new ArrayList<>(); for (AlignmentAnnotation ann : getAlignmentAnnotation()) { if ((calcId == null || (ann.getCalcId() != null && ann.getCalcId() @@ -1343,9 +1349,28 @@ public class SequenceGroup implements AnnotatedCollectionI private AnnotatedCollectionI context; /** + * Sets the alignment or group context for this group, and whether it is + * defined as a group + * + * @param ctx + * the context for the group + * @param defined + * whether the group is defined on the alignment or is just a + * selection + * @throws IllegalArgumentException + * if setting the context would result in a circular reference chain + */ + public void setContext(AnnotatedCollectionI ctx, boolean defined) + { + setContext(ctx); + this.isDefined = defined; + } + + /** * Sets the alignment or group context for this group * * @param ctx + * the context for the group * @throws IllegalArgumentException * if setting the context would result in a circular reference chain */ @@ -1375,6 +1400,11 @@ public class SequenceGroup implements AnnotatedCollectionI return context; } + public boolean isDefined() + { + return isDefined; + } + public void setColourScheme(ColourSchemeI scheme) { if (cs == null) diff --git a/src/jalview/viewmodel/AlignmentViewport.java b/src/jalview/viewmodel/AlignmentViewport.java index cdf0758..60cee46 100644 --- a/src/jalview/viewmodel/AlignmentViewport.java +++ b/src/jalview/viewmodel/AlignmentViewport.java @@ -92,9 +92,9 @@ public abstract class AlignmentViewport implements AlignViewportI, FeaturesDisplayedI featuresDisplayed = null; - protected Deque historyList = new ArrayDeque(); + protected Deque historyList = new ArrayDeque<>(); - protected Deque redoList = new ArrayDeque(); + protected Deque redoList = new ArrayDeque<>(); /** * alignment displayed in the viewport. Please use get/setter @@ -1113,7 +1113,6 @@ public abstract class AlignmentViewport implements AlignViewportI, public void setHiddenColumns(HiddenColumns hidden) { this.alignment.setHiddenColumns(hidden); - // this.colSel = colsel; } @Override @@ -1298,7 +1297,7 @@ public abstract class AlignmentViewport implements AlignViewportI, protected boolean showOccupancy = true; - private Map sequenceColours = new HashMap(); + private Map sequenceColours = new HashMap<>(); protected SequenceAnnotationOrder sortAnnotationsBy = null; @@ -1528,7 +1527,7 @@ public abstract class AlignmentViewport implements AlignViewportI, if (hiddenRepSequences == null) { - hiddenRepSequences = new Hashtable(); + hiddenRepSequences = new Hashtable<>(); } hiddenRepSequences.put(repSequence, sg); @@ -1736,7 +1735,7 @@ public abstract class AlignmentViewport implements AlignViewportI, @Override public List getVisibleRegionBoundaries(int min, int max) { - ArrayList regions = new ArrayList(); + ArrayList regions = new ArrayList<>(); int start = min; int end = max; @@ -1779,7 +1778,7 @@ public abstract class AlignmentViewport implements AlignViewportI, public List getVisibleAlignmentAnnotation( boolean selectedOnly) { - ArrayList ala = new ArrayList(); + ArrayList ala = new ArrayList<>(); AlignmentAnnotation[] aa; if ((aa = alignment.getAlignmentAnnotation()) != null) { @@ -2133,7 +2132,7 @@ public abstract class AlignmentViewport implements AlignViewportI, // intersect alignment annotation with alignment groups AlignmentAnnotation[] aan = alignment.getAlignmentAnnotation(); - List oldrfs = new ArrayList(); + List oldrfs = new ArrayList<>(); if (aan != null) { for (int an = 0; an < aan.length; an++) @@ -2846,8 +2845,7 @@ public abstract class AlignmentViewport implements AlignViewportI, selectionIsDefinedGroup = gps.contains(selectionGroup); } } - return selectionGroup.getContext() == alignment - || selectionIsDefinedGroup; + return selectionGroup.isDefined() || selectionIsDefinedGroup; } /** diff --git a/test/jalview/datamodel/SequenceGroupTest.java b/test/jalview/datamodel/SequenceGroupTest.java index 6e1c2db..7837b49 100644 --- a/test/jalview/datamodel/SequenceGroupTest.java +++ b/test/jalview/datamodel/SequenceGroupTest.java @@ -10,10 +10,10 @@ import static org.testng.Assert.fail; import jalview.schemes.NucleotideColourScheme; -import junit.extensions.PA; - import org.testng.annotations.Test; +import junit.extensions.PA; + public class SequenceGroupTest { @Test(groups={"Functional"}) @@ -121,13 +121,32 @@ public class SequenceGroupTest PA.setValue(sg2, "context", sg2); try { - sg3.setContext(sg2); // circular reference in sg2 + sg3.setContext(sg2, false); // circular reference in sg2 fail("Expected exception"); } catch (IllegalArgumentException e) { // expected assertNull(sg3.getContext()); } + + // test isDefined setting behaviour + sg2 = new SequenceGroup(); + sg1.setContext(null, false); + assertFalse(sg1.isDefined()); + + sg1.setContext(sg2, false); + assertFalse(sg1.isDefined()); + + sg1.setContext(sg2, true); + assertTrue(sg1.isDefined()); + + // setContext without defined parameter does not change isDefined + sg1.setContext(null); + assertTrue(sg1.isDefined()); + + sg1.setContext(null, false); + sg1.setContext(sg2); + assertFalse(sg1.isDefined()); } @Test(groups = { "Functional" }) diff --git a/test/jalview/gui/AlignViewportTest.java b/test/jalview/gui/AlignViewportTest.java index e6c16b7..4660842 100644 --- a/test/jalview/gui/AlignViewportTest.java +++ b/test/jalview/gui/AlignViewportTest.java @@ -160,7 +160,7 @@ public class AlignViewportTest acf2.addMap(s1, s1, new MapList(new int[] { 1, 4 }, new int[] { 4, 1 }, 1, 1)); - List mappings = new ArrayList(); + List mappings = new ArrayList<>(); mappings.add(acf1); mappings.add(acf2); af1.getViewport().getAlignment().setCodonFrames(mappings); @@ -217,11 +217,11 @@ public class AlignViewportTest acf3.addMap(cs2, cs2, new MapList(new int[] { 1, 12 }, new int[] { 1, 12 }, 1, 1)); - List mappings1 = new ArrayList(); + List mappings1 = new ArrayList<>(); mappings1.add(acf1); af1.getViewport().getAlignment().setCodonFrames(mappings1); - List mappings2 = new ArrayList(); + List mappings2 = new ArrayList<>(); mappings2.add(acf2); mappings2.add(acf3); af2.getViewport().getAlignment().setCodonFrames(mappings2); @@ -280,12 +280,12 @@ public class AlignViewportTest acf3.addMap(cs2, cs2, new MapList(new int[] { 1, 12 }, new int[] { 1, 12 }, 1, 1)); - List mappings1 = new ArrayList(); + List mappings1 = new ArrayList<>(); mappings1.add(acf1); mappings1.add(acf2); af1.getViewport().getAlignment().setCodonFrames(mappings1); - List mappings2 = new ArrayList(); + List mappings2 = new ArrayList<>(); mappings2.add(acf2); mappings2.add(acf3); af2.getViewport().getAlignment().setCodonFrames(mappings2); @@ -389,7 +389,8 @@ public class AlignViewportTest /** * Verify that setting the selection group has the side-effect of setting the - * context on the group, unless it already has one + * context on the group, unless it already has one, but does not change + * whether the group is defined or not. */ @Test(groups = { "Functional" }) public void testSetSelectionGroup() @@ -399,13 +400,21 @@ public class AlignViewportTest AlignViewport av = af.getViewport(); SequenceGroup sg1 = new SequenceGroup(); SequenceGroup sg2 = new SequenceGroup(); + SequenceGroup sg3 = new SequenceGroup(); av.setSelectionGroup(sg1); assertSame(sg1.getContext(), av.getAlignment()); // context set + assertFalse(sg1.isDefined()); // group not defined - sg2.setContext(sg1); + sg2.setContext(sg1, false); av.setSelectionGroup(sg2); + assertFalse(sg2.isDefined()); // unchanged assertSame(sg2.getContext(), sg1); // unchanged + + // create a defined group + sg3.setContext(av.getAlignment(), true); + av.setSelectionGroup(sg3); + assertTrue(sg3.isDefined()); // unchanged } /** * Verify that setting/clearing SHOW_OCCUPANCY preference adds or omits occupancy row from viewport -- 1.7.10.2