From: gmungoc Date: Wed, 24 May 2017 14:21:43 +0000 (+0100) Subject: Merge branch 'bug/JAL-2541cutWithFeatures' into features/JAL-2446NCList X-Git-Tag: Release_2_10_3b1~239 X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=f1fbc7674102f63dfe1bd156a2d19f3c658e35d5;hp=b44349fe3a5f2d70b74a1b110018a753a1036aca;p=jalview.git Merge branch 'bug/JAL-2541cutWithFeatures' into features/JAL-2446NCList --- diff --git a/src/jalview/commands/EditCommand.java b/src/jalview/commands/EditCommand.java index 98ac2d5..388c533 100644 --- a/src/jalview/commands/EditCommand.java +++ b/src/jalview/commands/EditCommand.java @@ -555,6 +555,7 @@ public class EditCommand implements CommandI command.oldds = new SequenceI[command.seqs.length]; } command.oldds[i] = oldds; + // FIXME JAL-2541 JAL-2526 get correct positions if on a gap adjustFeatures( command, i, @@ -1101,8 +1102,8 @@ public class EditCommand implements CommandI } } - final static void adjustFeatures(Edit command, int index, int i, int j, - boolean insert) + final static void adjustFeatures(Edit command, int index, final int i, + final int j, boolean insert) { SequenceI seq = command.seqs[index]; SequenceI sequence = seq.getDatasetSequence(); diff --git a/src/jalview/datamodel/Sequence.java b/src/jalview/datamodel/Sequence.java index af6592b..9f3e7b8 100755 --- a/src/jalview/datamodel/Sequence.java +++ b/src/jalview/datamodel/Sequence.java @@ -36,6 +36,8 @@ import java.util.Enumeration; import java.util.List; import java.util.Vector; +import com.stevesoft.pat.Regex; + import fr.orsay.lri.varna.models.rna.RNA; /** @@ -47,6 +49,11 @@ import fr.orsay.lri.varna.models.rna.RNA; */ public class Sequence extends ASequence implements SequenceI { + private static final Regex limitrx = new Regex( + "[/][0-9]{1,}[-][0-9]{1,}$"); + + private static final Regex endrx = new Regex("[0-9]{1,}$"); + SequenceI datasetSequence; String name; @@ -129,11 +136,6 @@ public class Sequence extends ASequence implements SequenceI checkValidRange(); } - com.stevesoft.pat.Regex limitrx = new com.stevesoft.pat.Regex( - "[/][0-9]{1,}[-][0-9]{1,}$"); - - com.stevesoft.pat.Regex endrx = new com.stevesoft.pat.Regex("[0-9]{1,}$"); - void parseId() { if (name == null) @@ -235,23 +237,28 @@ public class Sequence extends ASequence implements SequenceI protected void initSeqFrom(SequenceI seq, AlignmentAnnotation[] alAnnotation) { - { - char[] oseq = seq.getSequence(); - initSeqAndName(seq.getName(), Arrays.copyOf(oseq, oseq.length), - seq.getStart(), seq.getEnd()); - } + char[] oseq = seq.getSequence(); + initSeqAndName(seq.getName(), Arrays.copyOf(oseq, oseq.length), + seq.getStart(), seq.getEnd()); + description = seq.getDescription(); if (seq != datasetSequence) { setDatasetSequence(seq.getDatasetSequence()); } - if (datasetSequence == null && seq.getDBRefs() != null) + + /* + * only copy DBRefs and seqfeatures if we really are a dataset sequence + */ + if (datasetSequence == null) { - // only copy DBRefs and seqfeatures if we really are a dataset sequence - DBRefEntry[] dbr = seq.getDBRefs(); - for (int i = 0; i < dbr.length; i++) + if (seq.getDBRefs() != null) { - addDBRef(new DBRefEntry(dbr[i])); + DBRefEntry[] dbr = seq.getDBRefs(); + for (int i = 0; i < dbr.length; i++) + { + addDBRef(new DBRefEntry(dbr[i])); + } } if (seq.getSequenceFeatures() != null) { @@ -262,6 +269,7 @@ public class Sequence extends ASequence implements SequenceI } } } + if (seq.getAnnotation() != null) { AlignmentAnnotation[] sqann = seq.getAnnotation(); @@ -1245,11 +1253,11 @@ public class Sequence extends ASequence implements SequenceI return null; } - Vector subset = new Vector(); - Enumeration e = annotation.elements(); + Vector subset = new Vector(); + Enumeration e = annotation.elements(); while (e.hasMoreElements()) { - AlignmentAnnotation ann = (AlignmentAnnotation) e.nextElement(); + AlignmentAnnotation ann = e.nextElement(); if (ann.label != null && ann.label.equals(label)) { subset.addElement(ann); @@ -1264,7 +1272,7 @@ public class Sequence extends ASequence implements SequenceI e = subset.elements(); while (e.hasMoreElements()) { - anns[i++] = (AlignmentAnnotation) e.nextElement(); + anns[i++] = e.nextElement(); } subset.removeAllElements(); return anns; @@ -1335,10 +1343,10 @@ public class Sequence extends ASequence implements SequenceI // transfer PDB entries if (entry.getAllPDBEntries() != null) { - Enumeration e = entry.getAllPDBEntries().elements(); + Enumeration e = entry.getAllPDBEntries().elements(); while (e.hasMoreElements()) { - PDBEntry pdb = (PDBEntry) e.nextElement(); + PDBEntry pdb = e.nextElement(); addPDBId(pdb); } } diff --git a/test/jalview/commands/EditCommandTest.java b/test/jalview/commands/EditCommandTest.java index 3223042..8781a93 100644 --- a/test/jalview/commands/EditCommandTest.java +++ b/test/jalview/commands/EditCommandTest.java @@ -21,6 +21,7 @@ package jalview.commands; import static org.testng.AssertJUnit.assertEquals; +import static org.testng.AssertJUnit.assertNull; import static org.testng.AssertJUnit.assertSame; import jalview.commands.EditCommand.Action; @@ -28,11 +29,15 @@ import jalview.commands.EditCommand.Edit; import jalview.datamodel.Alignment; import jalview.datamodel.AlignmentI; import jalview.datamodel.Sequence; +import jalview.datamodel.SequenceFeature; import jalview.datamodel.SequenceI; import jalview.gui.JvOptionPane; +import java.util.Arrays; +import java.util.Comparator; import java.util.Map; +import org.testng.Assert; import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -45,6 +50,14 @@ import org.testng.annotations.Test; */ public class EditCommandTest { + /* + * compute n(n+1)/2 e.g. + * func(5) = 5 + 4 + 3 + 2 + 1 = 15 + */ + private static int func(int i) + { + return i * (i + 1) / 2; + } @BeforeClass(alwaysRun = true) public void setUpJvOptionPane() @@ -639,4 +652,228 @@ public class EditCommandTest assertEquals(ds2, unwound.get(ds2).getDatasetSequence()); assertEquals(ds3, unwound.get(ds3).getDatasetSequence()); } + + /** + * Test a cut action's relocation of sequence features + */ + @Test(groups = { "Functional" }) + public void testCut_withFeatures() + { + /* + * create sequence features before, after and overlapping + * a cut of columns/residues 4-7 + */ + SequenceI seq0 = seqs[0]; + seq0.addSequenceFeature(new SequenceFeature("before", "", 1, 3, 0f, + null)); + seq0.addSequenceFeature(new SequenceFeature("overlap left", "", 2, 6, + 0f, null)); + seq0.addSequenceFeature(new SequenceFeature("internal", "", 5, 6, 0f, + null)); + seq0.addSequenceFeature(new SequenceFeature("overlap right", "", 7, 8, + 0f, null)); + seq0.addSequenceFeature(new SequenceFeature("after", "", 8, 10, 0f, + null)); + + Edit ec = testee.new Edit(Action.CUT, seqs, 3, 4, al); // cols 3-6 base 0 + EditCommand.cut(ec, new AlignmentI[] { al }); + + SequenceFeature[] sfs = seq0.getSequenceFeatures(); + Arrays.sort(sfs, new Comparator() + { + @Override + public int compare(SequenceFeature o1, SequenceFeature o2) + { + return Integer.compare(o1.getBegin(), o2.getBegin()); + } + }); + assertEquals(4, sfs.length); // feature internal to cut has been deleted + SequenceFeature sf = sfs[0]; + assertEquals("before", sf.getType()); + assertEquals(1, sf.getBegin()); + assertEquals(3, sf.getEnd()); + sf = sfs[1]; + assertEquals("overlap left", sf.getType()); + assertEquals(2, sf.getBegin()); + assertEquals(3, sf.getEnd()); // truncated by cut + sf = sfs[2]; + assertEquals("overlap right", sf.getType()); + assertEquals(4, sf.getBegin()); // shifted left by cut + assertEquals(5, sf.getEnd()); // truncated by cut + sf = sfs[3]; + assertEquals("after", sf.getType()); + assertEquals(4, sf.getBegin()); // shifted left by cut + assertEquals(6, sf.getEnd()); // shifted left by cut + } + + /** + * Test a cut action's relocation of sequence features, with full coverage of + * all possible feature and cut locations for a 5-position ungapped sequence + */ + @Test(groups = { "Functional" }) + public void testCut_withFeatures_exhaustive() + { + /* + * create a sequence features on each subrange of 1-5 + */ + SequenceI seq0 = new Sequence("seq", "ABCDE"); + AlignmentI alignment = new Alignment(new SequenceI[] { seq0 }); + alignment.setDataset(null); + for (int from = 1; from <= seq0.getLength(); from++) + { + for (int to = from; to <= seq0.getLength(); to++) + { + String desc = String.format("%d-%d", from, to); + SequenceFeature sf = new SequenceFeature("test", desc, from, to, + 0f, + null); + sf.setValue("from", Integer.valueOf(from)); + sf.setValue("to", Integer.valueOf(to)); + seq0.addSequenceFeature(sf); + } + } + // sanity check + SequenceFeature[] sfs = seq0.getSequenceFeatures(); + assertEquals(func(5), sfs.length); + + /* + * now perform all possible cuts of subranges of 1-5 (followed by Undo) + * and validate the resulting remaining sequence features! + */ + SequenceI[] sqs = new SequenceI[] { seq0 }; + + // goal is to have this passing for all from/to values!! + // for (int from = 0; from < seq0.getLength(); from++) + // { + // for (int to = from; to < seq0.getLength(); to++) + for (int from = 1; from < 3; from++) + { + for (int to = 2; to < 3; to++) + { + testee.appendEdit(Action.CUT, sqs, from, (to - from + 1), + alignment, true); + + sfs = seq0.getSequenceFeatures(); + + /* + * confirm the number of features has reduced by the + * number of features within the cut region i.e. by + * func(length of cut) + */ + String msg = String.format("Cut %d-%d ", from, to); + if (to - from == 4) + { + // all columns cut + assertNull(sfs); + } + else + { + assertEquals(msg + "wrong number of features left", func(5) + - func(to - from + 1), sfs.length); + } + + /* + * inspect individual features + */ + if (sfs != null) + { + for (SequenceFeature sf : sfs) + { + checkFeatureRelocation(sf, from + 1, to + 1); + } + } + /* + * undo ready for next cut + */ + testee.undoCommand(new AlignmentI[] { alignment }); + assertEquals(func(5), seq0.getSequenceFeatures().length); + } + } + } + + /** + * Helper method to check a feature has been correctly relocated after a cut + * + * @param sf + * @param from + * start of cut (first residue cut) + * @param to + * end of cut (last residue cut) + */ + private void checkFeatureRelocation(SequenceFeature sf, int from, int to) + { + // TODO handle the gapped sequence case as well + int cutSize = to - from + 1; + int oldFrom = ((Integer) sf.getValue("from")).intValue(); + int oldTo = ((Integer) sf.getValue("to")).intValue(); + + String msg = String.format( + "Feature %s relocated to %d-%d after cut of %d-%d", + sf.getDescription(), sf.getBegin(), sf.getEnd(), from, to); + if (oldTo < from) + { + // before cut region so unchanged + assertEquals("1: " + msg, oldFrom, sf.getBegin()); + assertEquals("2: " + msg, oldTo, sf.getEnd()); + } + else if (oldFrom > to) + { + // follows cut region - shift by size of cut + assertEquals("3: " + msg, oldFrom - cutSize, sf.getBegin()); + assertEquals("4: " + msg, oldTo - cutSize, sf.getEnd()); + } + else if (oldFrom < from && oldTo > to) + { + // feature encloses cut region - shrink it right + assertEquals("5: " + msg, oldFrom, sf.getBegin()); + assertEquals("6: " + msg, oldTo - cutSize, sf.getEnd()); + } + else if (oldFrom < from) + { + // feature overlaps left side of cut region - truncated right + assertEquals("7: " + msg, from - 1, sf.getEnd()); + } + else if (oldTo > to) + { + // feature overlaps right side of cut region - truncated left + assertEquals("8: " + msg, from, sf.getBegin()); + assertEquals("9: " + msg, from + oldTo - to - 1, sf.getEnd()); + } + else + { + // feature internal to cut - should have been deleted! + Assert.fail(msg + " - should have been deleted"); + } + } + + /** + * Test a cut action's relocation of sequence features + */ + @Test(groups = { "Functional" }) + public void testCut_gappedWithFeatures() + { + /* + * create sequence features before, after and overlapping + * a cut of columns/residues 4-7 + */ + SequenceI seq0 = new Sequence("seq", "A-BCC"); + seq0.addSequenceFeature(new SequenceFeature("", "", 3, 4, 0f, + null)); + AlignmentI alignment = new Alignment(new SequenceI[] { seq0 }); + // cut columns of A-B + Edit ec = testee.new Edit(Action.CUT, seqs, 0, 3, alignment); // cols 0-3 + // base 0 + EditCommand.cut(ec, new AlignmentI[] { alignment }); + + /* + * feature on CC(3-4) should now be on CC(1-2) + */ + SequenceFeature[] sfs = seq0.getSequenceFeatures(); + assertEquals(1, sfs.length); + SequenceFeature sf = sfs[0]; + assertEquals(1, sf.getBegin()); + assertEquals(2, sf.getEnd()); + + // TODO add further cases including Undo - see JAL-2541 + } } diff --git a/test/jalview/datamodel/SequenceTest.java b/test/jalview/datamodel/SequenceTest.java index 586cc5d..ebf4857 100644 --- a/test/jalview/datamodel/SequenceTest.java +++ b/test/jalview/datamodel/SequenceTest.java @@ -23,6 +23,7 @@ package jalview.datamodel; import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.assertFalse; import static org.testng.AssertJUnit.assertNotNull; +import static org.testng.AssertJUnit.assertNotSame; import static org.testng.AssertJUnit.assertNull; import static org.testng.AssertJUnit.assertSame; import static org.testng.AssertJUnit.assertTrue; @@ -40,6 +41,8 @@ import java.util.Vector; import junit.extensions.PA; +import junit.extensions.PA; + import org.testng.Assert; import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeMethod; @@ -287,19 +290,129 @@ public class SequenceTest @Test(groups = { "Functional" }) public void testDeleteChars() { + /* + * internal delete + */ + SequenceI sq = new Sequence("test", "ABCDEF"); + assertNull(PA.getValue(sq, "datasetSequence")); + assertEquals(1, sq.getStart()); + assertEquals(6, sq.getEnd()); + sq.deleteChars(2, 3); + assertEquals("ABDEF", sq.getSequenceAsString()); + assertEquals(1, sq.getStart()); + assertEquals(5, sq.getEnd()); + assertNull(PA.getValue(sq, "datasetSequence")); + + /* + * delete at start + */ + sq = new Sequence("test", "ABCDEF"); + sq.deleteChars(0, 2); + assertEquals("CDEF", sq.getSequenceAsString()); + assertEquals(3, sq.getStart()); + assertEquals(6, sq.getEnd()); + assertNull(PA.getValue(sq, "datasetSequence")); + + /* + * delete at end + */ + sq = new Sequence("test", "ABCDEF"); + sq.deleteChars(4, 6); + assertEquals("ABCD", sq.getSequenceAsString()); + assertEquals(1, sq.getStart()); + assertEquals(4, sq.getEnd()); + assertNull(PA.getValue(sq, "datasetSequence")); + } + + @Test(groups = { "Functional" }) + public void testDeleteChars_withDbRefsAndFeatures() + { + /* + * internal delete - new dataset sequence created + * gets a copy of any dbrefs + */ SequenceI sq = new Sequence("test", "ABCDEF"); + sq.createDatasetSequence(); + DBRefEntry dbr1 = new DBRefEntry("Uniprot", "0", "a123"); + sq.addDBRef(dbr1); + Object ds = PA.getValue(sq, "datasetSequence"); + assertNotNull(ds); assertEquals(1, sq.getStart()); assertEquals(6, sq.getEnd()); sq.deleteChars(2, 3); assertEquals("ABDEF", sq.getSequenceAsString()); assertEquals(1, sq.getStart()); assertEquals(5, sq.getEnd()); + Object newDs = PA.getValue(sq, "datasetSequence"); + assertNotNull(newDs); + assertNotSame(ds, newDs); + assertNotNull(sq.getDBRefs()); + assertEquals(1, sq.getDBRefs().length); + assertNotSame(dbr1, sq.getDBRefs()[0]); + assertEquals(dbr1, sq.getDBRefs()[0]); + + /* + * internal delete with sequence features + * (failure case for JAL-2541) + */ + sq = new Sequence("test", "ABCDEF"); + sq.createDatasetSequence(); + SequenceFeature sf1 = new SequenceFeature("Cath", "desc", 2, 4, 2f, + "CathGroup"); + sq.addSequenceFeature(sf1); + ds = PA.getValue(sq, "datasetSequence"); + assertNotNull(ds); + assertEquals(1, sq.getStart()); + assertEquals(6, sq.getEnd()); + sq.deleteChars(2, 4); + assertEquals("ABEF", sq.getSequenceAsString()); + assertEquals(1, sq.getStart()); + assertEquals(4, sq.getEnd()); + newDs = PA.getValue(sq, "datasetSequence"); + assertNotNull(newDs); + assertNotSame(ds, newDs); + SequenceFeature[] sfs = sq.getSequenceFeatures(); + assertNotNull(sfs); + assertEquals(1, sfs.length); + assertNotSame(sf1, sfs[0]); + assertEquals(sf1, sfs[0]); + /* + * delete at start - no new dataset sequence created + * any sequence features remain as before + */ sq = new Sequence("test", "ABCDEF"); + sq.createDatasetSequence(); + ds = PA.getValue(sq, "datasetSequence"); + sf1 = new SequenceFeature("Cath", "desc", 2, 4, 2f, "CathGroup"); + sq.addSequenceFeature(sf1); sq.deleteChars(0, 2); assertEquals("CDEF", sq.getSequenceAsString()); assertEquals(3, sq.getStart()); assertEquals(6, sq.getEnd()); + assertSame(ds, PA.getValue(sq, "datasetSequence")); + sfs = sq.getSequenceFeatures(); + assertNotNull(sfs); + assertEquals(1, sfs.length); + assertSame(sf1, sfs[0]); + + /* + * delete at end - no new dataset sequence created + * any dbrefs remain as before + */ + sq = new Sequence("test", "ABCDEF"); + sq.createDatasetSequence(); + ds = PA.getValue(sq, "datasetSequence"); + dbr1 = new DBRefEntry("Uniprot", "0", "a123"); + sq.addDBRef(dbr1); + sq.deleteChars(4, 6); + assertEquals("ABCD", sq.getSequenceAsString()); + assertEquals(1, sq.getStart()); + assertEquals(4, sq.getEnd()); + assertSame(ds, PA.getValue(sq, "datasetSequence")); + assertNotNull(sq.getDBRefs()); + assertEquals(1, sq.getDBRefs().length); + assertSame(dbr1, sq.getDBRefs()[0]); } @Test(groups = { "Functional" })