From 1580169b8b5b244d380ec7b99a732dd34b0d454a Mon Sep 17 00:00:00 2001 From: gmungoc Date: Thu, 15 Jun 2017 14:06:15 +0100 Subject: [PATCH] JAL-2541 revised cut with features (work in progress) --- src/jalview/commands/EditCommand.java | 241 +++++++++++--------- src/jalview/datamodel/Sequence.java | 76 +++++- src/jalview/datamodel/SequenceI.java | 20 +- src/jalview/datamodel/features/FeatureStore.java | 39 ++-- .../datamodel/features/SequenceFeatures.java | 4 +- .../datamodel/features/SequenceFeaturesI.java | 10 +- src/jalview/ws/DBRefFetcher.java | 3 +- test/jalview/commands/EditCommandTest.java | 8 +- .../datamodel/features/FeatureStoreTest.java | 35 ++- .../datamodel/features/SequenceFeaturesTest.java | 35 ++- 10 files changed, 322 insertions(+), 149 deletions(-) diff --git a/src/jalview/commands/EditCommand.java b/src/jalview/commands/EditCommand.java index 9eaeb7a..243363e 100644 --- a/src/jalview/commands/EditCommand.java +++ b/src/jalview/commands/EditCommand.java @@ -20,12 +20,14 @@ */ package jalview.commands; +import jalview.analysis.AlignSeq; import jalview.datamodel.AlignmentAnnotation; import jalview.datamodel.AlignmentI; import jalview.datamodel.Annotation; import jalview.datamodel.Sequence; import jalview.datamodel.SequenceFeature; import jalview.datamodel.SequenceI; +import jalview.util.Comparison; import jalview.util.ReverseListIterator; import jalview.util.StringUtils; @@ -556,12 +558,21 @@ public class EditCommand implements CommandI } command.oldds[i] = oldds; // FIXME JAL-2541 JAL-2526 get correct positions if on a gap - adjustFeatures( - command, - i, - sequence.findPosition(command.position), - sequence.findPosition(command.position + command.number), - false); + List amendedFeatures = sequence + .adjustFeatures(command.position, command.position + + command.number - 1); + if (command.editedFeatures == null) + { + command.editedFeatures = new HashMap<>(); + } + command.editedFeatures.put(sequence, amendedFeatures); + // + // adjustFeatures( + // command, + // i, + // sequence.findPosition(command.position), + // sequence.findPosition(command.position + command.number), + // false); } } } @@ -596,7 +607,8 @@ public class EditCommand implements CommandI { newDSNeeded = false; newDSWasNeeded = command.oldds != null && command.oldds[i] != null; - if (command.seqs[i].getLength() < 1) + SequenceI sequence = command.seqs[i]; + if (sequence.getLength() < 1) { // ie this sequence was deleted, we need to // readd it to the alignment @@ -607,21 +619,21 @@ public class EditCommand implements CommandI { if (!(command.alIndex[i] < 0)) { - sequences.add(command.alIndex[i], command.seqs[i]); + sequences.add(command.alIndex[i], sequence); } } } else { - command.al.addSequence(command.seqs[i]); + command.al.addSequence(sequence); } seqWasDeleted = true; } - newstart = command.seqs[i].getStart(); - newend = command.seqs[i].getEnd(); + newstart = sequence.getStart(); + newend = sequence.getEnd(); tmp = new StringBuffer(); - tmp.append(command.seqs[i].getSequence()); + tmp.append(sequence.getSequence()); // Undo of a delete does not replace original dataset sequence on to // alignment sequence. @@ -641,16 +653,18 @@ public class EditCommand implements CommandI tmp.insert(command.position, command.string[i]); for (int s = 0; s < command.string[i].length; s++) { - if (jalview.schemes.ResidueProperties.aaIndex[command.string[i][s]] != 23) + // if (jalview.schemes.ResidueProperties.aaIndex[command.string[i][s]] + // != 23) + if (!Comparison.isGap(command.string[i][s])) { if (!newDSNeeded) { newDSNeeded = true; - start = command.seqs[i].findPosition(command.position); - end = command.seqs[i].findPosition(command.position + start = sequence.findPosition(command.position); + end = sequence.findPosition(command.position + command.number); } - if (command.seqs[i].getStart() == start) + if (sequence.getStart() == start) { newstart--; } @@ -663,12 +677,12 @@ public class EditCommand implements CommandI command.string[i] = null; } - command.seqs[i].setSequence(tmp.toString()); - command.seqs[i].setStart(newstart); - command.seqs[i].setEnd(newend); + sequence.setSequence(tmp.toString()); + sequence.setStart(newstart); + sequence.setEnd(newend); if (newDSNeeded) { - if (command.seqs[i].getDatasetSequence() != null) + if (sequence.getDatasetSequence() != null) { SequenceI ds; if (newDSWasNeeded) @@ -679,21 +693,20 @@ public class EditCommand implements CommandI { // make a new DS sequence // use new ds mechanism here - ds = new Sequence(command.seqs[i].getName(), - jalview.analysis.AlignSeq.extractGaps( - jalview.util.Comparison.GapChars, - command.seqs[i].getSequenceAsString()), - command.seqs[i].getStart(), command.seqs[i].getEnd()); - ds.setDescription(command.seqs[i].getDescription()); + String ungapped = AlignSeq.extractGaps(Comparison.GapChars, + sequence.getSequenceAsString()); + ds = new Sequence(sequence.getName(), ungapped, + sequence.getStart(), sequence.getEnd()); + ds.setDescription(sequence.getDescription()); } if (command.oldds == null) { command.oldds = new SequenceI[command.seqs.length]; } - command.oldds[i] = command.seqs[i].getDatasetSequence(); - command.seqs[i].setDatasetSequence(ds); + command.oldds[i] = sequence.getDatasetSequence(); + sequence.setDatasetSequence(ds); } - adjustFeatures(command, i, start, end, true); + undoCutFeatures(command, i, start, end); } } adjustAnnotations(command, true, seqWasDeleted, views); @@ -1104,8 +1117,8 @@ public class EditCommand implements CommandI } } - final static void adjustFeatures(Edit command, int index, final int i, - final int j, boolean insert) + final static void undoCutFeatures(Edit command, int index, final int i, + final int j) { SequenceI seq = command.seqs[index]; SequenceI sequence = seq.getDatasetSequence(); @@ -1114,87 +1127,101 @@ public class EditCommand implements CommandI sequence = seq; } - if (insert) + /* + * insert == true for an Undo of a Cut; restore the original features + * and delete the amended ones + */ + if (true) { + // TODO shift right features that lie to the right of the restored cut + // (add a start position parameter to SequenceFeatures.shift) + if (command.editedFeatures != null && command.editedFeatures.containsKey(seq)) { - sequence.setSequenceFeatures(command.editedFeatures.get(seq)); + for (SequenceFeature[] toRestore : command.editedFeatures.get(seq)) + { + sequence.addSequenceFeature(toRestore[0]); + if (toRestore[1] != null) + { + sequence.deleteFeature(toRestore[1]); + } + } } - return; } - List sf = sequence.getFeatures() - .getPositionalFeatures(); - - if (sf.isEmpty()) - { - return; - } - - List oldsf = new ArrayList(); - - int cSize = j - i; - - for (SequenceFeature feature : sf) - { - SequenceFeature copy = new SequenceFeature(feature); - - oldsf.add(copy); - - if (feature.getEnd() < i) - { - continue; - } - - if (feature.getBegin() > j) - { - int newBegin = copy.getBegin() - cSize; - int newEnd = copy.getEnd() - cSize; - SequenceFeature newSf = new SequenceFeature(feature, newBegin, - newEnd, feature.getFeatureGroup(), feature.getScore()); - sequence.deleteFeature(feature); - sequence.addSequenceFeature(newSf); - // feature.setBegin(newBegin); - // feature.setEnd(newEnd); - continue; - } - - int newBegin = feature.getBegin(); - int newEnd = feature.getEnd(); - if (newBegin >= i) - { - newBegin = i; - // feature.setBegin(i); - } - - if (newEnd < j) - { - newEnd = j - 1; - // feature.setEnd(j - 1); - } - newEnd = newEnd - cSize; - // feature.setEnd(feature.getEnd() - (cSize)); - - sequence.deleteFeature(feature); - if (newEnd >= newBegin) - { - sequence.addSequenceFeature(new SequenceFeature(feature, newBegin, - newEnd, feature.getFeatureGroup(), feature.getScore())); - } - // if (feature.getBegin() > feature.getEnd()) - // { - // sequence.deleteFeature(feature); - // } - } - - if (command.editedFeatures == null) - { - command.editedFeatures = new Hashtable>(); - } - - command.editedFeatures.put(seq, oldsf); + // List sf = sequence.getFeatures() + // .getPositionalFeatures(); + // + // if (sf.isEmpty()) + // { + // return; + // } + // + // List oldsf = new ArrayList(); + // + // int cSize = j - i; + // + // for (SequenceFeature feature : sf) + // { + // SequenceFeature copy = new SequenceFeature(feature); + // + // oldsf.add(copy); + // + // if (feature.getEnd() < i) + // { + // continue; + // } + // + // if (feature.getBegin() > j) + // { + // int newBegin = copy.getBegin() - cSize; + // int newEnd = copy.getEnd() - cSize; + // SequenceFeature newSf = new SequenceFeature(feature, newBegin, + // newEnd, feature.getFeatureGroup(), feature.getScore()); + // sequence.deleteFeature(feature); + // sequence.addSequenceFeature(newSf); + // // feature.setBegin(newBegin); + // // feature.setEnd(newEnd); + // continue; + // } + // + // int newBegin = feature.getBegin(); + // int newEnd = feature.getEnd(); + // if (newBegin >= i) + // { + // newBegin = i; + // // feature.setBegin(i); + // } + // + // if (newEnd < j) + // { + // newEnd = j - 1; + // // feature.setEnd(j - 1); + // } + // newEnd = newEnd - cSize; + // // feature.setEnd(feature.getEnd() - (cSize)); + // + // sequence.deleteFeature(feature); + // if (newEnd >= newBegin) + // { + // sequence.addSequenceFeature(new SequenceFeature(feature, newBegin, + // newEnd, feature.getFeatureGroup(), feature.getScore())); + // } + // // if (feature.getBegin() > feature.getEnd()) + // // { + // // sequence.deleteFeature(feature); + // // } + // } + // + // if (command.editedFeatures == null) + // { + // command.editedFeatures = new Hashtable>(); + // } + // + // command.editedFeatures.put(seq, oldsf); } @@ -1314,11 +1341,11 @@ public class EditCommand implements CommandI boolean fullAlignmentHeight = false; - Hashtable deletedAnnotationRows; + Map deletedAnnotationRows; - Hashtable deletedAnnotations; + Map deletedAnnotations; - Hashtable> editedFeatures; + Map> editedFeatures; AlignmentI al; diff --git a/src/jalview/datamodel/Sequence.java b/src/jalview/datamodel/Sequence.java index 96747e4..1da4fc9 100755 --- a/src/jalview/datamodel/Sequence.java +++ b/src/jalview/datamodel/Sequence.java @@ -44,10 +44,7 @@ import fr.orsay.lri.varna.models.rna.RNA; /** * - * Implements the SequenceI interface for a char[] based sequence object. - * - * @author $author$ - * @version $Revision$ + * Implements the SequenceI interface for a char[] based sequence object */ public class Sequence extends ASequence implements SequenceI { @@ -1856,4 +1853,75 @@ public class Sequence extends ASequence implements SequenceI return count; } + + @Override + public List adjustFeatures(int fromColumn, int toColumn) + { + List amended = new ArrayList<>(); + + if (toColumn < fromColumn) + { + return amended; + } + + synchronized (sequenceFeatureStore) + { + /* + * get features that overlap or span the cut region + */ + List overlaps = findFeatures(fromColumn, toColumn); + int cutWidth = toColumn - fromColumn + 1; + + /* + * get features that strictly follow the cut region, + * and shift them left by the width of the cut + */ + List follow = findFeatures(toColumn + 1, + Integer.MAX_VALUE); + follow.removeAll(overlaps); + for (SequenceFeature sf : follow) + { + SequenceFeature copy = new SequenceFeature(sf, sf.getBegin() + - cutWidth, sf.getEnd() - cutWidth, sf.getFeatureGroup(), + sf.getScore()); + deleteFeature(sf); + addSequenceFeature(copy); + } + + /* + * adjust start-end of overlapping features, and delete if enclosed by + * the cut, or a partially overlapping contact feature + */ + for (SequenceFeature sf : overlaps) + { + // TODO recode to compute newBegin, newEnd, isDelete + // then perform the action + int sfBegin = sf.getBegin(); + int sfEnd = sf.getEnd(); + int startCol = findIndex(sfBegin); + int endCol = findIndex(sfEnd); + if (startCol >= fromColumn && endCol <= toColumn) + { + // within cut region - delete feature + deleteFeature(sf); + amended.add(new SequenceFeature[] { sf, null }); + continue; + } + if (startCol < fromColumn && endCol > toColumn) + { + // feature spans cut region - shift end left + SequenceFeature copy = new SequenceFeature(sf, sf.getBegin(), + sf.getEnd() - cutWidth, sf.getFeatureGroup(), + sf.getScore()); + deleteFeature(sf); + addSequenceFeature(copy); + amended.add(new SequenceFeature[] { sf, copy }); + continue; + } + // todo partial overlap - delete if contact feature + } + } + + return amended; + } } diff --git a/src/jalview/datamodel/SequenceI.java b/src/jalview/datamodel/SequenceI.java index 6840df8..66a0638 100755 --- a/src/jalview/datamodel/SequenceI.java +++ b/src/jalview/datamodel/SequenceI.java @@ -523,5 +523,23 @@ public interface SequenceI extends ASequenceI * @param c1 * @param c2 */ - public int replace(char c1, char c2); + int replace(char c1, char c2); + + /** + * Adjusts position and extent of features to allow for cut of the specified + * (inclusive) column range. Returns a list of {originalFeature, + * amendedFeature} for + *
    + *
  • features that have been deleted (as within the cut) - amendedFeature is + * null
    • + *
  • truncated features (as overlapping or spanning the cut)
    • + *
+ * Contact features that overlap the cut region are deleted. Contact features + * that enclose the cut region are shortened. + * + * @param fromColumn + * @param toColumn + * @return + */ + List adjustFeatures(int fromColumn, int toColumn); } diff --git a/src/jalview/datamodel/features/FeatureStore.java b/src/jalview/datamodel/features/FeatureStore.java index 3e80966..4dd2ebc 100644 --- a/src/jalview/datamodel/features/FeatureStore.java +++ b/src/jalview/datamodel/features/FeatureStore.java @@ -999,13 +999,15 @@ public class FeatureStore } /** - * Adds the shift value to the start and end of all positional features. - * Returns true if at least one feature was updated, else false. + * Adds the shift amount to the start and end of all positional features whose + * start position is at or after fromPosition. Returns true if at least one + * feature was shifted, else false. * - * @param shift + * @param fromPosition + * @param shiftBy * @return */ - public synchronized boolean shiftFeatures(int shift) + public synchronized boolean shiftFeatures(int fromPosition, int shiftBy) { /* * Because begin and end are final fields (to ensure the data store's @@ -1015,21 +1017,24 @@ public class FeatureStore boolean modified = false; for (SequenceFeature sf : getPositionalFeatures()) { - modified = true; - int newBegin = sf.getBegin() + shift; - int newEnd = sf.getEnd() + shift; - - /* - * sanity check: don't shift left of the first residue - */ - if (newEnd > 0) + if (sf.getBegin() >= fromPosition) { - newBegin = Math.max(1, newBegin); - SequenceFeature sf2 = new SequenceFeature(sf, newBegin, newEnd, - sf.getFeatureGroup(), sf.getScore()); - addFeature(sf2); + modified = true; + int newBegin = sf.getBegin() + shiftBy; + int newEnd = sf.getEnd() + shiftBy; + + /* + * sanity check: don't shift left of the first residue + */ + if (newEnd > 0) + { + newBegin = Math.max(1, newBegin); + SequenceFeature sf2 = new SequenceFeature(sf, newBegin, newEnd, + sf.getFeatureGroup(), sf.getScore()); + addFeature(sf2); + } + delete(sf); } - delete(sf); } return modified; } diff --git a/src/jalview/datamodel/features/SequenceFeatures.java b/src/jalview/datamodel/features/SequenceFeatures.java index 8f6d496..3a927e4 100644 --- a/src/jalview/datamodel/features/SequenceFeatures.java +++ b/src/jalview/datamodel/features/SequenceFeatures.java @@ -482,12 +482,12 @@ public class SequenceFeatures implements SequenceFeaturesI * {@inheritDoc} */ @Override - public boolean shiftFeatures(int shift) + public boolean shiftFeatures(int fromPosition, int shiftBy) { boolean modified = false; for (FeatureStore fs : featureStore.values()) { - modified |= fs.shiftFeatures(shift); + modified |= fs.shiftFeatures(fromPosition, shiftBy); } return modified; } diff --git a/src/jalview/datamodel/features/SequenceFeaturesI.java b/src/jalview/datamodel/features/SequenceFeaturesI.java index 58beca2..40beae3 100644 --- a/src/jalview/datamodel/features/SequenceFeaturesI.java +++ b/src/jalview/datamodel/features/SequenceFeaturesI.java @@ -195,10 +195,12 @@ public interface SequenceFeaturesI float getMaximumScore(String type, boolean positional); /** - * Adds the shift amount to the start and end of all positional features, - * returning true if at least one feature was shifted, else false + * Adds the shift amount to the start and end of all positional features whose + * start position is at or after fromPosition. Returns true if at least one + * feature was shifted, else false. * - * @param shift + * @param fromPosition + * @param shiftBy */ - abstract boolean shiftFeatures(int shift); + boolean shiftFeatures(int fromPosition, int shiftBy); } \ No newline at end of file diff --git a/src/jalview/ws/DBRefFetcher.java b/src/jalview/ws/DBRefFetcher.java index 8c8a717..0817864 100644 --- a/src/jalview/ws/DBRefFetcher.java +++ b/src/jalview/ws/DBRefFetcher.java @@ -702,7 +702,8 @@ public class DBRefFetcher implements Runnable int startShift = absStart - sequenceStart + 1; if (startShift != 0) { - modified |= sequence.getFeatures().shiftFeatures(startShift); + modified |= sequence.getFeatures().shiftFeatures(1, + startShift); } } } diff --git a/test/jalview/commands/EditCommandTest.java b/test/jalview/commands/EditCommandTest.java index 155f00e..7f1a432 100644 --- a/test/jalview/commands/EditCommandTest.java +++ b/test/jalview/commands/EditCommandTest.java @@ -736,13 +736,9 @@ public class EditCommandTest */ 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 from = 0; from < seq0.getLength(); from++) { - for (int to = 2; to < 3; to++) + for (int to = from; to < seq0.getLength(); to++) { testee.appendEdit(Action.CUT, sqs, from, (to - from + 1), alignment, true); diff --git a/test/jalview/datamodel/features/FeatureStoreTest.java b/test/jalview/datamodel/features/FeatureStoreTest.java index f5be818..375ff1b 100644 --- a/test/jalview/datamodel/features/FeatureStoreTest.java +++ b/test/jalview/datamodel/features/FeatureStoreTest.java @@ -2,6 +2,7 @@ package jalview.datamodel.features; 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.datamodel.SequenceFeature; @@ -774,7 +775,7 @@ public class FeatureStoreTest public void testShiftFeatures() { FeatureStore fs = new FeatureStore(); - assertFalse(fs.shiftFeatures(1)); + assertFalse(fs.shiftFeatures(0, 1)); // nothing to do SequenceFeature sf1 = new SequenceFeature("Cath", "", 2, 5, 0f, null); fs.addFeature(sf1); @@ -790,9 +791,9 @@ public class FeatureStoreTest fs.addFeature(sf4); /* - * shift features right by 5 + * shift all features right by 5 */ - assertTrue(fs.shiftFeatures(5)); + assertTrue(fs.shiftFeatures(0, 5)); // non-positional features untouched: List nonPos = fs.getNonPositionalFeatures(); @@ -818,7 +819,7 @@ public class FeatureStoreTest * feature at [7-10] should be removed * feature at [13-19] should become [1-4] */ - assertTrue(fs.shiftFeatures(-15)); + assertTrue(fs.shiftFeatures(0, -15)); pos = fs.getPositionalFeatures(); assertEquals(pos.size(), 2); SequenceFeatures.sortFeatures(pos, true); @@ -826,5 +827,31 @@ public class FeatureStoreTest assertEquals(pos.get(0).getEnd(), 4); assertEquals(pos.get(1).getBegin(), 13); assertEquals(pos.get(1).getEnd(), 22); + + /* + * shift right by 4 from position 2 onwards + * feature at [1-4] unchanged, feature at [13-22] shifts + */ + assertTrue(fs.shiftFeatures(2, 4)); + pos = fs.getPositionalFeatures(); + assertEquals(pos.size(), 2); + SequenceFeatures.sortFeatures(pos, true); + assertEquals(pos.get(0).getBegin(), 1); + assertEquals(pos.get(0).getEnd(), 4); + assertEquals(pos.get(1).getBegin(), 17); + assertEquals(pos.get(1).getEnd(), 26); + + /* + * shift right by 4 from position 18 onwards + * should be no change + */ + SequenceFeature f1 = pos.get(0); + SequenceFeature f2 = pos.get(1); + assertFalse(fs.shiftFeatures(18, 4)); // no update + pos = fs.getPositionalFeatures(); + assertEquals(pos.size(), 2); + SequenceFeatures.sortFeatures(pos, true); + assertSame(pos.get(0), f1); + assertSame(pos.get(1), f2); } } diff --git a/test/jalview/datamodel/features/SequenceFeaturesTest.java b/test/jalview/datamodel/features/SequenceFeaturesTest.java index b7b52a7..57ed86e 100644 --- a/test/jalview/datamodel/features/SequenceFeaturesTest.java +++ b/test/jalview/datamodel/features/SequenceFeaturesTest.java @@ -1143,7 +1143,7 @@ public class SequenceFeaturesTest public void testShiftFeatures() { SequenceFeatures store = new SequenceFeatures(); - assertFalse(store.shiftFeatures(1)); + assertFalse(store.shiftFeatures(0, 1)); SequenceFeature sf1 = new SequenceFeature("Cath", "", 2, 5, 0f, null); store.add(sf1); @@ -1161,7 +1161,7 @@ public class SequenceFeaturesTest /* * shift features right by 5 */ - assertTrue(store.shiftFeatures(5)); + assertTrue(store.shiftFeatures(0, 5)); // non-positional features untouched: List nonPos = store.getNonPositionalFeatures(); @@ -1190,7 +1190,7 @@ public class SequenceFeaturesTest * feature at [7-10] should be removed * feature at [13-19] should become [1-4] */ - assertTrue(store.shiftFeatures(-15)); + assertTrue(store.shiftFeatures(0, -15)); pos = store.getPositionalFeatures(); assertEquals(pos.size(), 2); SequenceFeatures.sortFeatures(pos, true); @@ -1200,6 +1200,35 @@ public class SequenceFeaturesTest assertEquals(pos.get(1).getBegin(), 13); assertEquals(pos.get(1).getEnd(), 22); assertEquals(pos.get(1).getType(), "Disulfide bond"); + + /* + * shift right by 4 from column 2 + * feature at [1-4] should be unchanged + * feature at [13-22] should become [17-26] + */ + assertTrue(store.shiftFeatures(2, 4)); + pos = store.getPositionalFeatures(); + assertEquals(pos.size(), 2); + SequenceFeatures.sortFeatures(pos, true); + assertEquals(pos.get(0).getBegin(), 1); + assertEquals(pos.get(0).getEnd(), 4); + assertEquals(pos.get(0).getType(), "Metal"); + assertEquals(pos.get(1).getBegin(), 17); + assertEquals(pos.get(1).getEnd(), 26); + assertEquals(pos.get(1).getType(), "Disulfide bond"); + + /* + * shift right from column 18 + * should be no updates + */ + SequenceFeature f1 = pos.get(0); + SequenceFeature f2 = pos.get(1); + assertFalse(store.shiftFeatures(18, 6)); + pos = store.getPositionalFeatures(); + assertEquals(pos.size(), 2); + SequenceFeatures.sortFeatures(pos, true); + assertSame(pos.get(0), f1); + assertSame(pos.get(1), f2); } @Test(groups = "Functional") -- 1.7.10.2