From: Jim Procter Date: Mon, 17 Dec 2018 16:00:33 +0000 (+0000) Subject: Merge branch 'task/JAL-3174_fixed_filename_tests' into develop X-Git-Tag: Release_2_11_1_0~97 X-Git-Url: http://source.jalview.org/gitweb/?p=jalview.git;a=commitdiff_plain;h=698646a270bc76f9a68d81f72fbaed80d4384c41;hp=6a2c35d5cde338e1eb9ee851f2a5a7a00ddee819 Merge branch 'task/JAL-3174_fixed_filename_tests' into develop --- diff --git a/help/html/releases.html b/help/html/releases.html index af417a7..eb13ab3 100755 --- a/help/html/releases.html +++ b/help/html/releases.html @@ -71,25 +71,43 @@ li:before {
2.11.0
- 8/09/2018 + 29/01/2019
-
- - -
-
- - -
+
+ + +
+
+ + + Editing + +
diff --git a/src/jalview/commands/EditCommand.java b/src/jalview/commands/EditCommand.java index cac843f..1a227c5 100644 --- a/src/jalview/commands/EditCommand.java +++ b/src/jalview/commands/EditCommand.java @@ -20,12 +20,16 @@ */ package jalview.commands; +import jalview.analysis.AlignSeq; import jalview.datamodel.AlignmentAnnotation; import jalview.datamodel.AlignmentI; import jalview.datamodel.Annotation; +import jalview.datamodel.Range; import jalview.datamodel.Sequence; import jalview.datamodel.SequenceFeature; import jalview.datamodel.SequenceI; +import jalview.datamodel.features.SequenceFeaturesI; +import jalview.util.Comparison; import jalview.util.ReverseListIterator; import jalview.util.StringUtils; @@ -114,7 +118,7 @@ public class EditCommand implements CommandI public abstract Action getUndoAction(); }; - private List edits = new ArrayList(); + private List edits = new ArrayList<>(); String description; @@ -330,20 +334,8 @@ public class EditCommand implements CommandI int position, int number, AlignmentI al, boolean performEdit, AlignmentI[] views) { - Edit edit = new Edit(command, seqs, position, number, - al.getGapCharacter()); - if (al.getHeight() == seqs.length) - { - edit.al = al; - edit.fullAlignmentHeight = true; - } - - addEdit(edit); - - if (performEdit) - { - performEdit(edit, views); - } + Edit edit = new Edit(command, seqs, position, number, al); + appendEdit(edit, al, performEdit, views); } /** @@ -534,39 +526,62 @@ public class EditCommand implements CommandI command.string[i] = sequence.getSequence(command.position, command.position + command.number); SequenceI oldds = sequence.getDatasetSequence(); - if (command.oldds != null && command.oldds[i] != null) - { - // we are redoing an undone cut. - sequence.setDatasetSequence(null); - } - sequence.deleteChars(command.position, + Range cutPositions = sequence.findPositions(command.position + 1, command.position + command.number); + boolean cutIsInternal = cutPositions != null + && sequence.getStart() != cutPositions + .getBegin() && sequence.getEnd() != cutPositions.getEnd(); + + /* + * perform the cut; if this results in a new dataset sequence, add + * that to the alignment dataset + */ + SequenceI ds = sequence.getDatasetSequence(); + sequence.deleteChars(command.position, command.position + + command.number); + if (command.oldds != null && command.oldds[i] != null) { - // oldds entry contains the cut dataset sequence. + /* + * we are Redoing a Cut, or Undoing a Paste - so + * oldds entry contains the cut dataset sequence, + * with sequence features in expected place + */ sequence.setDatasetSequence(command.oldds[i]); command.oldds[i] = oldds; } else { - // modify the oldds if necessary + /* + * new cut operation: save the dataset sequence + * so it can be restored in an Undo + */ + if (command.oldds == null) + { + command.oldds = new SequenceI[command.seqs.length]; + } + command.oldds[i] = oldds;// todo not if !cutIsInternal? + + // do we need to edit sequence features for new sequence ? if (oldds != sequence.getDatasetSequence() - || sequence.getFeatures().hasFeatures()) + || (cutIsInternal + && sequence.getFeatures().hasFeatures())) + // todo or just test cutIsInternal && cutPositions != null ? { - if (command.oldds == null) + if (cutPositions != null) { - command.oldds = new SequenceI[command.seqs.length]; + cutFeatures(command, sequence, cutPositions.getBegin(), + cutPositions.getEnd(), cutIsInternal); } - 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); } } + SequenceI newDs = sequence.getDatasetSequence(); + if (newDs != ds && command.al != null + && command.al.getDataset() != null + && !command.al.getDataset().getSequences().contains(newDs)) + { + command.al.getDataset().addSequence(newDs); + } } if (sequence.getLength() < 1) @@ -588,21 +603,19 @@ public class EditCommand implements CommandI */ static void paste(Edit command, AlignmentI[] views) { - StringBuffer tmp; - boolean newDSNeeded; - boolean newDSWasNeeded; - int newstart, newend; boolean seqWasDeleted = false; - int start = 0, end = 0; for (int i = 0; i < command.seqs.length; i++) { - newDSNeeded = false; - newDSWasNeeded = command.oldds != null && command.oldds[i] != null; - if (command.seqs[i].getLength() < 1) + boolean newDSNeeded = false; + boolean newDSWasNeeded = command.oldds != null + && command.oldds[i] != null; + SequenceI sequence = command.seqs[i]; + if (sequence.getLength() < 1) { - // ie this sequence was deleted, we need to - // readd it to the alignment + /* + * sequence was deleted; re-add it to the alignment + */ if (command.alIndex[i] < command.al.getHeight()) { List sequences; @@ -610,68 +623,78 @@ 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(); + int newStart = sequence.getStart(); + int newEnd = sequence.getEnd(); - tmp = new StringBuffer(); - tmp.append(command.seqs[i].getSequence()); + StringBuilder tmp = new StringBuilder(); + tmp.append(sequence.getSequence()); // Undo of a delete does not replace original dataset sequence on to // alignment sequence. + int start = 0; + int length = 0; + if (command.string != null && command.string[i] != null) { if (command.position >= tmp.length()) { // This occurs if padding is on, and residues // are removed from end of alignment - int length = command.position - tmp.length(); - while (length > 0) + int len = command.position - tmp.length(); + while (len > 0) { tmp.append(command.gapChar); - length--; + len--; } } 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 (!Comparison.isGap(command.string[i][s])) { + length++; if (!newDSNeeded) { newDSNeeded = true; - start = command.seqs[i].findPosition(command.position); - end = command.seqs[i] - .findPosition(command.position + command.number); + start = sequence.findPosition(command.position); + // end = sequence + // .findPosition(command.position + command.number); } - if (command.seqs[i].getStart() == start) + if (sequence.getStart() == start) { - newstart--; + newStart--; } else { - newend++; + newEnd++; } } } 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); + + /* + * command and Undo share the same dataset sequence if cut was + * at start or end of sequence + */ + boolean sameDatasetSequence = false; if (newDSNeeded) { - if (command.seqs[i].getDatasetSequence() != null) + if (sequence.getDatasetSequence() != null) { SequenceI ds; if (newDSWasNeeded) @@ -682,21 +705,29 @@ 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(); + sameDatasetSequence = ds == sequence.getDatasetSequence(); + ds.setSequenceFeatures(sequence.getSequenceFeatures()); + if (!sameDatasetSequence && command.al.getDataset() != null) + { + // delete 'undone' sequence from alignment dataset + command.al.getDataset() + .deleteSequence(sequence.getDatasetSequence()); + } + sequence.setDatasetSequence(ds); } - adjustFeatures(command, i, start, end, true); + undoCutFeatures(command, command.seqs[i], start, length, + sameDatasetSequence); } } adjustAnnotations(command, true, seqWasDeleted, views); @@ -706,7 +737,7 @@ public class EditCommand implements CommandI static void replace(Edit command) { - StringBuffer tmp; + StringBuilder tmp; String oldstring; int start = command.position; int end = command.number; @@ -719,6 +750,7 @@ public class EditCommand implements CommandI { boolean newDSWasNeeded = command.oldds != null && command.oldds[i] != null; + boolean newStartEndWasNeeded = command.oldStartEnd!=null && command.oldStartEnd[i]!=null; /** * cut addHistoryItem(new EditCommand("Cut Sequences", EditCommand.CUT, @@ -731,49 +763,147 @@ public class EditCommand implements CommandI * EditCommand.PASTE, sequences, 0, alignment.getWidth(), alignment) ); * */ + + Range beforeEditedPositions = command.seqs[i].findPositions(1, start); + Range afterEditedPositions = command.seqs[i] + .findPositions(end + 1, command.seqs[i].getLength()); + oldstring = command.seqs[i].getSequenceAsString(); - tmp = new StringBuffer(oldstring.substring(0, start)); + tmp = new StringBuilder(oldstring.substring(0, start)); tmp.append(command.string[i]); - String nogaprep = jalview.analysis.AlignSeq.extractGaps( - jalview.util.Comparison.GapChars, + String nogaprep = AlignSeq.extractGaps(Comparison.GapChars, new String(command.string[i])); - int ipos = command.seqs[i].findPosition(start) - - command.seqs[i].getStart(); - tmp.append(oldstring.substring(end)); + if (end < oldstring.length()) + { + tmp.append(oldstring.substring(end)); + } + // stash end prior to updating the sequence object so we can save it if + // need be. + Range oldstartend = new Range(command.seqs[i].getStart(), + command.seqs[i].getEnd()); command.seqs[i].setSequence(tmp.toString()); - command.string[i] = oldstring.substring(start, end).toCharArray(); - String nogapold = jalview.analysis.AlignSeq.extractGaps( - jalview.util.Comparison.GapChars, + command.string[i] = oldstring + .substring(start, Math.min(end, oldstring.length())) + .toCharArray(); + String nogapold = AlignSeq.extractGaps(Comparison.GapChars, new String(command.string[i])); + if (!nogaprep.toLowerCase().equals(nogapold.toLowerCase())) { - if (newDSWasNeeded) + // we may already have dataset and limits stashed... + if (newDSWasNeeded || newStartEndWasNeeded) { + if (newDSWasNeeded) + { + // then just switch the dataset sequence SequenceI oldds = command.seqs[i].getDatasetSequence(); command.seqs[i].setDatasetSequence(command.oldds[i]); command.oldds[i] = oldds; + } + if (newStartEndWasNeeded) + { + Range newStart = command.oldStartEnd[i]; + command.oldStartEnd[i] = oldstartend; + command.seqs[i].setStart(newStart.getBegin()); + command.seqs[i].setEnd(newStart.getEnd()); + } } - else + else { - if (command.oldds == null) + // decide if we need a new dataset sequence or modify start/end + // first edit the original dataset sequence string + SequenceI oldds = command.seqs[i].getDatasetSequence(); + String osp = oldds.getSequenceAsString(); + int beforeStartOfEdit = -oldds.getStart() + 1 + + (beforeEditedPositions == null + ? ((afterEditedPositions != null) + ? afterEditedPositions.getBegin() - 1 + : oldstartend.getBegin() + + nogapold.length()) + : beforeEditedPositions.getEnd() + ); + int afterEndOfEdit = -oldds.getStart() + 1 + + ((afterEditedPositions == null) + ? oldstartend.getEnd() + : afterEditedPositions.getBegin() - 1); + String fullseq = osp.substring(0, + beforeStartOfEdit) + + nogaprep + + osp.substring(afterEndOfEdit); + + // and check if new sequence data is different.. + if (!fullseq.equalsIgnoreCase(osp)) { - command.oldds = new SequenceI[command.seqs.length]; - } - command.oldds[i] = command.seqs[i].getDatasetSequence(); - SequenceI newds = new Sequence( - command.seqs[i].getDatasetSequence()); - String fullseq, osp = newds.getSequenceAsString(); - fullseq = osp.substring(0, ipos) + nogaprep - + osp.substring(ipos + nogaprep.length()); - newds.setSequence(fullseq.toUpperCase()); - // TODO: JAL-1131 ensure newly created dataset sequence is added to - // the set of - // dataset sequences associated with the alignment. - // TODO: JAL-1131 fix up any annotation associated with new dataset - // sequence to ensure that original sequence/annotation relationships - // are preserved. - command.seqs[i].setDatasetSequence(newds); + // old ds and edited ds are different, so + // create the new dataset sequence + SequenceI newds = new Sequence(oldds); + newds.setSequence(fullseq); + + if (command.oldds == null) + { + command.oldds = new SequenceI[command.seqs.length]; + } + command.oldds[i] = command.seqs[i].getDatasetSequence(); + + // And preserve start/end for good-measure + if (command.oldStartEnd == null) + { + command.oldStartEnd = new Range[command.seqs.length]; + } + command.oldStartEnd[i] = oldstartend; + // TODO: JAL-1131 ensure newly created dataset sequence is added to + // the set of + // dataset sequences associated with the alignment. + // TODO: JAL-1131 fix up any annotation associated with new dataset + // sequence to ensure that original sequence/annotation + // relationships + // are preserved. + command.seqs[i].setDatasetSequence(newds); + } + else + { + if (command.oldStartEnd == null) + { + command.oldStartEnd = new Range[command.seqs.length]; + } + command.oldStartEnd[i] = new Range(command.seqs[i].getStart(), + command.seqs[i].getEnd()); + if (beforeEditedPositions != null + && afterEditedPositions == null) + { + // modification at end + command.seqs[i].setEnd( + beforeEditedPositions.getEnd() + nogaprep.length() + - nogapold.length()); + } + else if (afterEditedPositions != null + && beforeEditedPositions == null) + { + // modification at start + command.seqs[i].setStart( + afterEditedPositions.getBegin() - nogaprep.length()); + } + else + { + // edit covered both start and end. Here we can only guess the + // new + // start/end + String nogapalseq = AlignSeq.extractGaps(Comparison.GapChars, + command.seqs[i].getSequenceAsString().toUpperCase()); + int newStart = command.seqs[i].getDatasetSequence() + .getSequenceAsString().indexOf(nogapalseq); + if (newStart == -1) + { + throw new Error( + "Implementation Error: could not locate start/end " + + "in dataset sequence after an edit of the sequence string"); + } + int newEnd = newStart + nogapalseq.length() - 1; + command.seqs[i].setStart(newStart); + command.seqs[i].setEnd(newEnd); + } + } } } tmp = null; @@ -789,7 +919,7 @@ public class EditCommand implements CommandI if (modifyVisibility && !insert) { // only occurs if a sequence was added or deleted. - command.deletedAnnotationRows = new Hashtable(); + command.deletedAnnotationRows = new Hashtable<>(); } if (command.fullAlignmentHeight) { @@ -892,8 +1022,7 @@ public class EditCommand implements CommandI } // and then duplicate added annotation on every other alignment // view - for (int vnum = 0; views != null - && vnum < views.length; vnum++) + for (int vnum = 0; views != null && vnum < views.length; vnum++) { if (views[vnum] != command.al) { @@ -948,7 +1077,7 @@ public class EditCommand implements CommandI if (!insert) { - command.deletedAnnotations = new Hashtable(); + command.deletedAnnotations = new Hashtable<>(); } int aSize; @@ -1109,98 +1238,97 @@ public class EditCommand implements CommandI } } - final static void adjustFeatures(Edit command, int index, final int i, - final int j, boolean insert) + /** + * Restores features to the state before a Cut. + *
    + *
  • re-add any features deleted by the cut
    • + *
  • remove any truncated features created by the cut
    • + *
  • shift right any features to the right of the cut
    • + *
+ * + * @param command + * the Cut command + * @param seq + * the sequence the Cut applied to + * @param start + * the start residue position of the cut + * @param length + * the number of residues cut + * @param sameDatasetSequence + * true if dataset sequence and frame of reference were left + * unchanged by the Cut + */ + final static void undoCutFeatures(Edit command, SequenceI seq, + final int start, final int length, boolean sameDatasetSequence) { - SequenceI seq = command.seqs[index]; SequenceI sequence = seq.getDatasetSequence(); if (sequence == null) { sequence = seq; } - if (insert) + /* + * shift right features that lie to the right of the restored cut (but not + * if dataset sequence unchanged - so coordinates were changed by Cut) + */ + if (!sameDatasetSequence) { - if (command.editedFeatures != null - && command.editedFeatures.containsKey(seq)) + /* + * shift right all features right of and not + * contiguous with the cut position + */ + seq.getFeatures().shiftFeatures(start + 1, length); + + /* + * shift right any features that start at the cut position, + * unless they were truncated + */ + List sfs = seq.getFeatures().findFeatures(start, + start); + for (SequenceFeature sf : sfs) { - sequence.setSequenceFeatures(command.editedFeatures.get(seq)); + if (sf.getBegin() == start) + { + if (!command.truncatedFeatures.containsKey(seq) + || !command.truncatedFeatures.get(seq).contains(sf)) + { + /* + * feature was shifted left to cut position (not truncated), + * so shift it back right + */ + SequenceFeature shifted = new SequenceFeature(sf, sf.getBegin() + + length, sf.getEnd() + length, sf.getFeatureGroup(), + sf.getScore()); + seq.addSequenceFeature(shifted); + seq.deleteFeature(sf); + } + } } - - return; } - List sf = sequence.getFeatures() - .getPositionalFeatures(); - - if (sf.isEmpty()) - { - return; - } - - List oldsf = new ArrayList(); - - int cSize = j - i; - - for (SequenceFeature feature : sf) + /* + * restore any features that were deleted or truncated + */ + if (command.deletedFeatures != null + && command.deletedFeatures.containsKey(seq)) { - 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) + for (SequenceFeature deleted : command.deletedFeatures.get(seq)) { - newEnd = j - 1; - // feature.setEnd(j - 1); + sequence.addSequenceFeature(deleted); } - 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) + /* + * delete any truncated features + */ + if (command.truncatedFeatures != null + && command.truncatedFeatures.containsKey(seq)) { - command.editedFeatures = new Hashtable>(); + for (SequenceFeature amended : command.truncatedFeatures.get(seq)) + { + sequence.deleteFeature(amended); + } } - - command.editedFeatures.put(seq, oldsf); - } /** @@ -1233,7 +1361,7 @@ public class EditCommand implements CommandI */ public Map priorState(boolean forUndo) { - Map result = new HashMap(); + Map result = new HashMap<>(); if (getEdits() == null) { return result; @@ -1266,7 +1394,7 @@ public class EditCommand implements CommandI * Work backwards through the edit list, deriving the sequences before each * was applied. The final result is the sequence set before any edits. */ - Iterator editList = new ReverseListIterator(getEdits()); + Iterator editList = new ReverseListIterator<>(getEdits()); while (editList.hasNext()) { Edit oldEdit = editList.next(); @@ -1315,29 +1443,45 @@ public class EditCommand implements CommandI public class Edit { - public SequenceI[] oldds; + private SequenceI[] oldds; + + /** + * start and end of sequence prior to edit + */ + private Range[] oldStartEnd; + + private boolean fullAlignmentHeight = false; - boolean fullAlignmentHeight = false; + private Map deletedAnnotationRows; - Hashtable deletedAnnotationRows; + private Map deletedAnnotations; - Hashtable deletedAnnotations; + /* + * features deleted by the cut (re-add on Undo) + * (including the original of any shortened features) + */ + private Map> deletedFeatures; - Hashtable> editedFeatures; + /* + * shortened features added by the cut (delete on Undo) + */ + private Map> truncatedFeatures; - AlignmentI al; + private AlignmentI al; - Action command; + final private Action command; char[][] string; SequenceI[] seqs; - int[] alIndex; + private int[] alIndex; - int position, number; + private int position; - char gapChar; + private int number; + + private char gapChar; public Edit(Action cmd, SequenceI[] sqs, int pos, int count, char gap) @@ -1352,11 +1496,8 @@ public class EditCommand implements CommandI Edit(Action cmd, SequenceI[] sqs, int pos, int count, AlignmentI align) { - this.gapChar = align.getGapCharacter(); - this.command = cmd; - this.seqs = sqs; - this.position = pos; - this.number = count; + this(cmd, sqs, pos, count, align.getGapCharacter()); + this.al = align; alIndex = new int[sqs.length]; @@ -1368,22 +1509,26 @@ public class EditCommand implements CommandI fullAlignmentHeight = (align.getHeight() == sqs.length); } + /** + * Constructor given a REPLACE command and the replacement string + * + * @param cmd + * @param sqs + * @param pos + * @param count + * @param align + * @param replace + */ Edit(Action cmd, SequenceI[] sqs, int pos, int count, AlignmentI align, String replace) { - this.command = cmd; - this.seqs = sqs; - this.position = pos; - this.number = count; - this.al = align; - this.gapChar = align.getGapCharacter(); + this(cmd, sqs, pos, count, align); + string = new char[sqs.length][]; for (int i = 0; i < sqs.length; i++) { string[i] = replace.toCharArray(); } - - fullAlignmentHeight = (align.getHeight() == sqs.length); } public SequenceI[] getSequences() @@ -1427,7 +1572,163 @@ public class EditCommand implements CommandI } else { - return new ReverseListIterator(getEdits()); + return new ReverseListIterator<>(getEdits()); + } + } + + /** + * Adjusts features for Cut, and saves details of changes made to allow Undo + *
    + *
  • features left of the cut are unchanged
    • + *
  • features right of the cut are shifted left
    • + *
  • features internal to the cut region are deleted
    • + *
  • features that overlap or span the cut are shortened
    • + *
  • the originals of any deleted or shortened features are saved, to re-add + * on Undo
    • + *
  • any added (shortened) features are saved, to delete on Undo
    • + *
+ * + * @param command + * @param seq + * @param fromPosition + * @param toPosition + * @param cutIsInternal + */ + protected static void cutFeatures(Edit command, SequenceI seq, + int fromPosition, int toPosition, boolean cutIsInternal) + { + /* + * if the cut is at start or end of sequence + * then we don't modify the sequence feature store + */ + if (!cutIsInternal) + { + return; + } + List added = new ArrayList<>(); + List removed = new ArrayList<>(); + + SequenceFeaturesI featureStore = seq.getFeatures(); + if (toPosition < fromPosition || featureStore == null) + { + return; + } + + int cutStartPos = fromPosition; + int cutEndPos = toPosition; + int cutWidth = cutEndPos - cutStartPos + 1; + + synchronized (featureStore) + { + /* + * get features that overlap the cut region + */ + List toAmend = featureStore.findFeatures( + cutStartPos, cutEndPos); + + /* + * add any contact features that span the cut region + * (not returned by findFeatures) + */ + for (SequenceFeature contact : featureStore.getContactFeatures()) + { + if (contact.getBegin() < cutStartPos + && contact.getEnd() > cutEndPos) + { + toAmend.add(contact); + } + } + + /* + * adjust start-end of overlapping features; + * delete features enclosed by the cut; + * delete partially overlapping contact features + */ + for (SequenceFeature sf : toAmend) + { + int sfBegin = sf.getBegin(); + int sfEnd = sf.getEnd(); + int newBegin = sfBegin; + int newEnd = sfEnd; + boolean toDelete = false; + boolean follows = false; + + if (sfBegin >= cutStartPos && sfEnd <= cutEndPos) + { + /* + * feature lies within cut region - delete it + */ + toDelete = true; + } + else if (sfBegin < cutStartPos && sfEnd > cutEndPos) + { + /* + * feature spans cut region - left-shift the end + */ + newEnd -= cutWidth; + } + else if (sfEnd <= cutEndPos) + { + /* + * feature overlaps left of cut region - truncate right + */ + newEnd = cutStartPos - 1; + if (sf.isContactFeature()) + { + toDelete = true; + } + } + else if (sfBegin >= cutStartPos) + { + /* + * remaining case - feature overlaps right + * truncate left, adjust end of feature + */ + newBegin = cutIsInternal ? cutStartPos : cutEndPos + 1; + newEnd = newBegin + sfEnd - cutEndPos - 1; + if (sf.isContactFeature()) + { + toDelete = true; + } + } + + seq.deleteFeature(sf); + if (!follows) + { + removed.add(sf); + } + if (!toDelete) + { + SequenceFeature copy = new SequenceFeature(sf, newBegin, newEnd, + sf.getFeatureGroup(), sf.getScore()); + seq.addSequenceFeature(copy); + if (!follows) + { + added.add(copy); + } + } + } + + /* + * and left shift any features lying to the right of the cut region + */ + + featureStore.shiftFeatures(cutEndPos + 1, -cutWidth); + } + + /* + * save deleted and amended features, so that Undo can + * re-add or delete them respectively + */ + if (command.deletedFeatures == null) + { + command.deletedFeatures = new HashMap<>(); + } + if (command.truncatedFeatures == null) + { + command.truncatedFeatures = new HashMap<>(); } + command.deletedFeatures.put(seq, removed); + command.truncatedFeatures.put(seq, added); } } diff --git a/src/jalview/datamodel/Sequence.java b/src/jalview/datamodel/Sequence.java index 33de452..f555855 100755 --- a/src/jalview/datamodel/Sequence.java +++ b/src/jalview/datamodel/Sequence.java @@ -43,10 +43,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 { @@ -797,6 +794,7 @@ public class Sequence extends ASequence implements SequenceI * preserve end residue column provided cursor was valid */ int endColumn = isValidCursor(cursor) ? cursor.lastColumnPosition : 0; + if (residuePos == this.end) { endColumn = column; @@ -833,8 +831,7 @@ public class Sequence extends ASequence implements SequenceI /* * move left or right to find pos from hint.position */ - int col = curs.columnPosition - 1; // convert from base 1 to 0-based array - // index + int col = curs.columnPosition - 1; // convert from base 1 to base 0 int newPos = curs.residuePosition; int delta = newPos > pos ? -1 : 1; @@ -1263,12 +1260,13 @@ public class Sequence extends ASequence implements SequenceI boolean createNewDs = false; // TODO: take a (second look) at the dataset creation validation method for // the very large sequence case + int startIndex = findIndex(start) - 1; int endIndex = findIndex(end) - 1; int startDeleteColumn = -1; // for dataset sequence deletions int deleteCount = 0; - for (int s = i; s < j; s++) + for (int s = i; s < j && s < sequence.length; s++) { if (Comparison.isGap(sequence[s])) { @@ -1913,6 +1911,15 @@ public class Sequence extends ASequence implements SequenceI List result = getFeatures().findFeatures(startPos, endPos, types); + if (datasetSequence != null) + { + result = datasetSequence.getFeatures().findFeatures(startPos, endPos, + types); + } + else + { + result = sequenceFeatureStore.findFeatures(startPos, endPos, types); + } /* * if end column is gapped, endPos may be to the right, diff --git a/src/jalview/datamodel/SequenceI.java b/src/jalview/datamodel/SequenceI.java index 8dce31e..48615f0 100755 --- a/src/jalview/datamodel/SequenceI.java +++ b/src/jalview/datamodel/SequenceI.java @@ -206,11 +206,14 @@ public interface SequenceI extends ASequenceI public int findPosition(int i); /** - * Returns the from-to sequence positions (start..) for the given column - * positions (1..), or null if no residues are included in the range + * Returns the sequence positions for first and last residues lying within the + * given column positions [fromColum,toColumn] (where columns are numbered + * from 1), or null if no residues are included in the range * * @param fromColum + * - first column base 1 * @param toColumn + * - last column, base 1 * @return */ public Range findPositions(int fromColum, int toColumn); diff --git a/src/jalview/datamodel/features/FeatureStore.java b/src/jalview/datamodel/features/FeatureStore.java index 02ce1c5..951b7a5 100644 --- a/src/jalview/datamodel/features/FeatureStore.java +++ b/src/jalview/datamodel/features/FeatureStore.java @@ -1047,13 +1047,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 @@ -1063,21 +1065,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 727d3ef..8f965b4 100644 --- a/src/jalview/datamodel/features/SequenceFeatures.java +++ b/src/jalview/datamodel/features/SequenceFeatures.java @@ -472,13 +472,22 @@ 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; } -} \ No newline at end of file + + /** + * {@inheritDoc} + */ + @Override + public void deleteAll() + { + featureStore.clear(); + } +} diff --git a/src/jalview/datamodel/features/SequenceFeaturesI.java b/src/jalview/datamodel/features/SequenceFeaturesI.java index 31712b9..ca0283e 100644 --- a/src/jalview/datamodel/features/SequenceFeaturesI.java +++ b/src/jalview/datamodel/features/SequenceFeaturesI.java @@ -215,10 +215,17 @@ 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); -} \ No newline at end of file + boolean shiftFeatures(int fromPosition, int shiftBy); + + /** + * Deletes all positional and non-positional features + */ + void deleteAll(); +} diff --git a/src/jalview/ws/DBRefFetcher.java b/src/jalview/ws/DBRefFetcher.java index 061e70c..41e4d0d 100644 --- a/src/jalview/ws/DBRefFetcher.java +++ b/src/jalview/ws/DBRefFetcher.java @@ -670,7 +670,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..2160657 100644 --- a/test/jalview/commands/EditCommandTest.java +++ b/test/jalview/commands/EditCommandTest.java @@ -21,8 +21,8 @@ package jalview.commands; import static org.testng.AssertJUnit.assertEquals; -import static org.testng.AssertJUnit.assertNull; import static org.testng.AssertJUnit.assertSame; +import static org.testng.AssertJUnit.assertTrue; import jalview.commands.EditCommand.Action; import jalview.commands.EditCommand.Edit; @@ -34,6 +34,8 @@ import jalview.datamodel.SequenceI; import jalview.datamodel.features.SequenceFeatures; import jalview.gui.JvOptionPane; +import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.Map; @@ -50,6 +52,22 @@ import org.testng.annotations.Test; */ public class EditCommandTest { + private static Comparator BY_DESCRIPTION = new Comparator() + { + + @Override + public int compare(SequenceFeature o1, SequenceFeature o2) + { + return o1.getDescription().compareTo(o2.getDescription()); + } + }; + + private EditCommand testee; + + private SequenceI[] seqs; + + private Alignment al; + /* * compute n(n+1)/2 e.g. * func(5) = 5 + 4 + 3 + 2 + 1 = 15 @@ -66,12 +84,6 @@ public class EditCommandTest JvOptionPane.setMockResponse(JvOptionPane.CANCEL_OPTION); } - private EditCommand testee; - - private SequenceI[] seqs; - - private Alignment al; - @BeforeMethod(alwaysRun = true) public void setUp() { @@ -141,18 +153,23 @@ public class EditCommandTest } /** - * Test a Paste action, where this adds sequences to an alignment. + * Test a Paste action, followed by Undo and Redo */ @Test(groups = { "Functional" }, enabled = false) - // TODO fix so it works - public void testPaste_addToAlignment() + public void testPaste_undo_redo() { + // TODO code this test properly, bearing in mind that: + // Paste action requires something on the clipboard (Cut/Copy) + // - EditCommand.paste doesn't add sequences to the alignment + // ... that is done in AlignFrame.paste() + // ... unless as a Redo + // ... + SequenceI[] newSeqs = new SequenceI[2]; newSeqs[0] = new Sequence("newseq0", "ACEFKL"); newSeqs[1] = new Sequence("newseq1", "JWMPDH"); - Edit ec = testee.new Edit(Action.PASTE, newSeqs, 0, al.getWidth(), al); - EditCommand.paste(ec, new AlignmentI[] { al }); + new EditCommand("Paste", Action.PASTE, newSeqs, 0, al.getWidth(), al); assertEquals(6, al.getSequences().size()); assertEquals("1234567890", seqs[3].getSequenceAsString()); assertEquals("ACEFKL", seqs[4].getSequenceAsString()); @@ -262,12 +279,123 @@ public class EditCommandTest { // seem to need a dataset sequence on the edited sequence here seqs[1].createDatasetSequence(); - new EditCommand("", Action.REPLACE, "ZXY", new SequenceI[] { seqs[1] }, + assertEquals("fghjklmnopq", seqs[1].getSequenceAsString()); + // NB command.number holds end position for a Replace command + new EditCommand("", Action.REPLACE, "Z-xY", new SequenceI[] { seqs[1] }, 4, 8, al); assertEquals("abcdefghjk", seqs[0].getSequenceAsString()); + assertEquals("fghjZ-xYopq", seqs[1].getSequenceAsString()); + assertEquals("fghjZxYopq", + seqs[1].getDatasetSequence().getSequenceAsString()); assertEquals("qrstuvwxyz", seqs[2].getSequenceAsString()); assertEquals("1234567890", seqs[3].getSequenceAsString()); - seqs[1] = new Sequence("seq1", "fghjZXYnopq"); + } + + /** + * Test the replace command (used to manually edit a sequence) + */ + @Test(groups = { "Functional" }) + public void testReplace_withGaps() + { + SequenceI seq = new Sequence("seq", "ABC--DEF"); + seq.createDatasetSequence(); + assertEquals("ABCDEF", seq.getDatasetSequence().getSequenceAsString()); + assertEquals(1, seq.getStart()); + assertEquals(6, seq.getEnd()); + + /* + * replace C- with XYZ + * NB arg4 = start column of selection for edit (base 0) + * arg5 = column after end of selection for edit + */ + EditCommand edit = new EditCommand("", Action.REPLACE, "xyZ", + new SequenceI[] + { seq }, 2, + 4, al); + assertEquals("ABxyZ-DEF", seq.getSequenceAsString()); + assertEquals(1, seq.getStart()); + assertEquals(8, seq.getEnd()); + assertEquals("ABxyZDEF", + seq.getDatasetSequence().getSequenceAsString()); + assertEquals(8, seq.getDatasetSequence().getEnd()); + + /* + * undo the edit + */ + AlignmentI[] views = new AlignmentI[] + { new Alignment(new SequenceI[] { seq }) }; + edit.undoCommand(views); + + assertEquals("ABC--DEF", seq.getSequenceAsString()); + assertEquals("ABCDEF", seq.getDatasetSequence().getSequenceAsString()); + assertEquals(1, seq.getStart()); + assertEquals(6, seq.getEnd()); + assertEquals(6, seq.getDatasetSequence().getEnd()); + + /* + * redo the edit + */ + edit.doCommand(views); + + assertEquals("ABxyZ-DEF", seq.getSequenceAsString()); + assertEquals(1, seq.getStart()); + assertEquals(8, seq.getEnd()); + assertEquals("ABxyZDEF", + seq.getDatasetSequence().getSequenceAsString()); + assertEquals(8, seq.getDatasetSequence().getEnd()); + + } + + /** + * Test replace command when it doesn't cause a sequence edit (see comment in + */ + @Test(groups = { "Functional" }) + public void testReplaceFirstResiduesWithGaps() + { + // test replace when gaps are inserted at start. Start/end should change + // w.r.t. original edited sequence. + SequenceI dsseq = seqs[1].getDatasetSequence(); + EditCommand edit = new EditCommand("", Action.REPLACE, "----", + new SequenceI[] + { seqs[1] }, 0, 4, al); + + // trimmed start + assertEquals("----klmnopq", seqs[1].getSequenceAsString()); + // and ds is preserved + assertTrue(dsseq == seqs[1].getDatasetSequence()); + // and it is unchanged + assertEquals("fghjklmnopq", dsseq.getSequenceAsString()); + // and that alignment sequence start has been adjusted + assertEquals(5, seqs[1].getStart()); + assertEquals(11, seqs[1].getEnd()); + + AlignmentI[] views = new AlignmentI[] { new Alignment(seqs) }; + // and undo + edit.undoCommand(views); + + // dataset sequence unchanged + assertTrue(dsseq == seqs[1].getDatasetSequence()); + // restore sequence + assertEquals("fghjklmnopq", seqs[1].getSequenceAsString()); + // and start/end numbering also restored + assertEquals(1, seqs[1].getStart()); + assertEquals(11, seqs[1].getEnd()); + + // now redo + edit.undoCommand(views); + + // and repeat asserts for the original edit + + // trimmed start + assertEquals("----klmnopq", seqs[1].getSequenceAsString()); + // and ds is preserved + assertTrue(dsseq == seqs[1].getDatasetSequence()); + // and it is unchanged + assertEquals("fghjklmnopq", dsseq.getSequenceAsString()); + // and that alignment sequence start has been adjusted + assertEquals(5, seqs[1].getStart()); + assertEquals(11, seqs[1].getEnd()); + } /** @@ -663,7 +791,7 @@ public class EditCommandTest * create sequence features before, after and overlapping * a cut of columns/residues 4-7 */ - SequenceI seq0 = seqs[0]; + SequenceI seq0 = seqs[0]; // abcdefghjk/1-10 seq0.addSequenceFeature(new SequenceFeature("before", "", 1, 3, 0f, null)); seq0.addSequenceFeature(new SequenceFeature("overlap left", "", 2, 6, @@ -675,29 +803,52 @@ public class EditCommandTest seq0.addSequenceFeature(new SequenceFeature("after", "", 8, 10, 0f, null)); + /* + * add some contact features + */ + SequenceFeature internalContact = new SequenceFeature("disulphide bond", "", 5, + 6, 0f, null); + seq0.addSequenceFeature(internalContact); // should get deleted + SequenceFeature overlapLeftContact = new SequenceFeature( + "disulphide bond", "", 2, 6, 0f, null); + seq0.addSequenceFeature(overlapLeftContact); // should get deleted + SequenceFeature overlapRightContact = new SequenceFeature( + "disulphide bond", "", 5, 8, 0f, null); + seq0.addSequenceFeature(overlapRightContact); // should get deleted + SequenceFeature spanningContact = new SequenceFeature( + "disulphide bond", "", 2, 9, 0f, null); + seq0.addSequenceFeature(spanningContact); // should get shortened 3' + + /* + * cut columns 3-6 (base 0), residues d-g 4-7 + */ Edit ec = testee.new Edit(Action.CUT, seqs, 3, 4, al); // cols 3-6 base 0 EditCommand.cut(ec, new AlignmentI[] { al }); List sfs = seq0.getSequenceFeatures(); SequenceFeatures.sortFeatures(sfs, true); - assertEquals(4, sfs.size()); // feature internal to cut has been deleted + assertEquals(5, sfs.size()); // features internal to cut were deleted SequenceFeature sf = sfs.get(0); assertEquals("before", sf.getType()); assertEquals(1, sf.getBegin()); assertEquals(3, sf.getEnd()); sf = sfs.get(1); + assertEquals("disulphide bond", sf.getType()); + assertEquals(2, sf.getBegin()); + assertEquals(5, sf.getEnd()); // truncated by cut + sf = sfs.get(2); assertEquals("overlap left", sf.getType()); assertEquals(2, sf.getBegin()); assertEquals(3, sf.getEnd()); // truncated by cut - sf = sfs.get(2); - assertEquals("overlap right", sf.getType()); - assertEquals(4, sf.getBegin()); // shifted left by cut - assertEquals(5, sf.getEnd()); // truncated by cut sf = sfs.get(3); assertEquals("after", sf.getType()); assertEquals(4, sf.getBegin()); // shifted left by cut assertEquals(6, sf.getEnd()); // shifted left by cut + sf = sfs.get(4); + assertEquals("overlap right", sf.getType()); + assertEquals(4, sf.getBegin()); // shifted left by cut + assertEquals(4, sf.getEnd()); // truncated by cut } /** @@ -711,16 +862,28 @@ public class EditCommandTest * create a sequence features on each subrange of 1-5 */ SequenceI seq0 = new Sequence("seq", "ABCDE"); + int start = 8; + int end = 12; + seq0.setStart(start); + seq0.setEnd(end); AlignmentI alignment = new Alignment(new SequenceI[] { seq0 }); alignment.setDataset(null); - for (int from = 1; from <= seq0.getLength(); from++) + + /* + * create a new alignment with shared dataset sequence + */ + AlignmentI copy = new Alignment( + new SequenceI[] + { alignment.getDataset().getSequenceAt(0).deriveSequence() }); + SequenceI copySeq0 = copy.getSequenceAt(0); + + for (int from = start; from <= end; from++) { - for (int to = from; to <= seq0.getLength(); to++) + for (int to = from; to <= end; to++) { String desc = String.format("%d-%d", from, to); SequenceFeature sf = new SequenceFeature("test", desc, from, to, - 0f, - null); + 0f, null); sf.setValue("from", Integer.valueOf(from)); sf.setValue("to", Integer.valueOf(to)); seq0.addSequenceFeature(sf); @@ -729,109 +892,234 @@ public class EditCommandTest // sanity check List sfs = seq0.getSequenceFeatures(); assertEquals(func(5), sfs.size()); + assertEquals(sfs, copySeq0.getSequenceFeatures()); + String copySequenceFeatures = copySeq0.getSequenceFeatures().toString(); /* - * now perform all possible cuts of subranges of 1-5 (followed by Undo) + * now perform all possible cuts of subranges of columns 1-5 * 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 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); + EditCommand ec = new EditCommand("Cut", Action.CUT, sqs, from, (to + - from + 1), alignment); + final String msg = String.format("Cut %d-%d ", from + 1, to + 1); + boolean newDatasetSequence = copySeq0.getDatasetSequence() != seq0 + .getDatasetSequence(); + + verifyCut(seq0, from, to, msg, start); + + /* + * verify copy alignment dataset sequence unaffected + */ + assertEquals("Original dataset sequence was modified", + copySequenceFeatures, + copySeq0.getSequenceFeatures().toString()); + /* + * verify any new dataset sequence was added to the + * alignment dataset + */ + assertEquals("Wrong Dataset size after " + msg, + newDatasetSequence ? 2 : 1, + alignment.getDataset().getHeight()); + + /* + * undo and verify all restored + */ + AlignmentI[] views = new AlignmentI[] { alignment }; + ec.undoCommand(views); sfs = seq0.getSequenceFeatures(); + assertEquals("After undo of " + msg, func(5), sfs.size()); + verifyUndo(from, to, sfs); + + /* + * verify copy alignment dataset sequence still unaffected + * and alignment dataset has shrunk (if it was added to) + */ + assertEquals("Original dataset sequence was modified", + copySequenceFeatures, + copySeq0.getSequenceFeatures().toString()); + assertEquals("Wrong Dataset size after Undo of " + msg, 1, + alignment.getDataset().getHeight()); /* - * confirm the number of features has reduced by the - * number of features within the cut region i.e. by - * func(length of cut) + * redo and verify */ - 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.size()); - } + ec.doCommand(views); + verifyCut(seq0, from, to, msg, start); /* - * inspect individual features + * verify copy alignment dataset sequence unaffected + * and any new dataset sequence readded to alignment dataset */ - if (sfs != null) - { - for (SequenceFeature sf : sfs) - { - checkFeatureRelocation(sf, from + 1, to + 1); - } - } + assertEquals("Original dataset sequence was modified", + copySequenceFeatures, + copySeq0.getSequenceFeatures().toString()); + assertEquals("Wrong Dataset size after Redo of " + msg, + newDatasetSequence ? 2 : 1, + alignment.getDataset().getHeight()); + /* * undo ready for next cut */ - testee.undoCommand(new AlignmentI[] { alignment }); - assertEquals(func(5), seq0.getSequenceFeatures().size()); + ec.undoCommand(views); + + /* + * final verify that copy alignment dataset sequence is still unaffected + * and that alignment dataset has shrunk + */ + assertEquals("Original dataset sequence was modified", + copySequenceFeatures, + copySeq0.getSequenceFeatures().toString()); + assertEquals("Wrong Dataset size after final Undo of " + msg, 1, + alignment.getDataset().getHeight()); } } } /** + * Verify by inspection that the sequence features left on the sequence after + * a cut match the expected results. The trick to this is that we can parse + * each feature's original start-end positions from its description. + * + * @param seq0 + * @param from + * @param to + * @param msg + * @param seqStart + */ + protected void verifyCut(SequenceI seq0, int from, int to, + final String msg, int seqStart) + { + List sfs; + sfs = seq0.getSequenceFeatures(); + + Collections.sort(sfs, BY_DESCRIPTION); + + /* + * confirm the number of features has reduced by the + * number of features within the cut region i.e. by + * func(length of cut); exception is a cut at start or end of sequence, + * which retains the original coordinates, dataset sequence + * and all its features + */ + boolean datasetRetained = from == 0 || to == 4; + if (datasetRetained) + { + // dataset and all features retained + assertEquals(msg, func(5), sfs.size()); + } + else if (to - from == 4) + { + // all columns were cut + assertTrue(sfs.isEmpty()); + } + else + { + // failure in checkFeatureRelocation is more informative! + assertEquals(msg + "wrong number of features left", func(5) + - func(to - from + 1), sfs.size()); + } + + /* + * inspect individual features + */ + for (SequenceFeature sf : sfs) + { + verifyFeatureRelocation(sf, from + 1, to + 1, !datasetRetained, + seqStart); + } + } + + /** + * Check that after Undo, every feature has start/end that match its original + * "start" and "end" properties + * + * @param from + * @param to + * @param sfs + */ + protected void verifyUndo(int from, int to, List sfs) + { + for (SequenceFeature sf : sfs) + { + final int oldFrom = ((Integer) sf.getValue("from")).intValue(); + final int oldTo = ((Integer) sf.getValue("to")).intValue(); + String msg = String.format( + "Undo cut of [%d-%d], feature at [%d-%d] ", from + 1, to + 1, + oldFrom, oldTo); + assertEquals(msg + "start", oldFrom, sf.getBegin()); + assertEquals(msg + "end", oldTo, sf.getEnd()); + } + } + + /** * Helper method to check a feature has been correctly relocated after a cut * * @param sf * @param from - * start of cut (first residue cut) + * start of cut (first residue cut 1..) * @param to - * end of cut (last residue cut) + * end of cut (last residue cut 1..) + * @param newDataset + * @param seqStart */ - private void checkFeatureRelocation(SequenceFeature sf, int from, int to) + private void verifyFeatureRelocation(SequenceFeature sf, int from, int to, + boolean newDataset, int seqStart) { // 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(); + final int oldFrom = ((Integer) sf.getValue("from")).intValue(); + final int oldTo = ((Integer) sf.getValue("to")).intValue(); + final int oldFromPosition = oldFrom - seqStart + 1; // 1.. + final int oldToPosition = oldTo - seqStart + 1; // 1.. 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) + if (!newDataset) + { + // dataset retained with all features unchanged + assertEquals("0: " + msg, oldFrom, sf.getBegin()); + assertEquals("0: " + msg, oldTo, sf.getEnd()); + } + else if (oldToPosition < from) { // before cut region so unchanged assertEquals("1: " + msg, oldFrom, sf.getBegin()); assertEquals("2: " + msg, oldTo, sf.getEnd()); } - else if (oldFrom > to) + else if (oldFromPosition > to) { // follows cut region - shift by size of cut - assertEquals("3: " + msg, oldFrom - cutSize, sf.getBegin()); - assertEquals("4: " + msg, oldTo - cutSize, sf.getEnd()); + assertEquals("3: " + msg, newDataset ? oldFrom - cutSize : oldFrom, + sf.getBegin()); + assertEquals("4: " + msg, newDataset ? oldTo - cutSize : oldTo, + sf.getEnd()); } - else if (oldFrom < from && oldTo > to) + else if (oldFromPosition < from && oldToPosition > 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) + else if (oldFromPosition < from) { // feature overlaps left side of cut region - truncated right - assertEquals("7: " + msg, from - 1, sf.getEnd()); + assertEquals("7: " + msg, from - 1 + seqStart - 1, sf.getEnd()); } - else if (oldTo > to) + else if (oldToPosition > 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()); + assertEquals("8: " + msg, newDataset ? from + seqStart - 1 : to + 1, + sf.getBegin()); + assertEquals("9: " + msg, newDataset ? from + oldTo - to - 1 : oldTo, + sf.getEnd()); } else { @@ -844,30 +1132,32 @@ public class EditCommandTest * Test a cut action's relocation of sequence features */ @Test(groups = { "Functional" }) - public void testCut_gappedWithFeatures() + public void testCut_withFeatures5prime() { + SequenceI seq0 = new Sequence("seq/8-11", "A-BCC"); + seq0.createDatasetSequence(); + assertEquals(8, seq0.getStart()); + seq0.addSequenceFeature(new SequenceFeature("", "", 10, 11, 0f, + null)); + SequenceI[] seqsArray = new SequenceI[] { seq0 }; + AlignmentI alignment = new Alignment(seqsArray); + /* - * create sequence features before, after and overlapping - * a cut of columns/residues 4-7 + * cut columns of A-B; same dataset sequence is retained, aligned sequence + * start becomes 10 */ - 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 + Edit ec = testee.new Edit(Action.CUT, seqsArray, 0, 3, alignment); EditCommand.cut(ec, new AlignmentI[] { alignment }); /* - * feature on CC(3-4) should now be on CC(1-2) + * feature on CC(10-11) should still be on CC(10-11) */ + assertSame(seq0, alignment.getSequenceAt(0)); + assertEquals(10, seq0.getStart()); List sfs = seq0.getSequenceFeatures(); assertEquals(1, sfs.size()); SequenceFeature sf = sfs.get(0); - assertEquals(1, sf.getBegin()); - assertEquals(2, sf.getEnd()); - - // TODO add further cases including Undo - see JAL-2541 + assertEquals(10, sf.getBegin()); + assertEquals(11, sf.getEnd()); } } diff --git a/test/jalview/datamodel/SequenceTest.java b/test/jalview/datamodel/SequenceTest.java index 79bb2bb..9629b6f 100644 --- a/test/jalview/datamodel/SequenceTest.java +++ b/test/jalview/datamodel/SequenceTest.java @@ -290,6 +290,61 @@ public class SequenceTest assertEquals(0, sq.findIndex(2)); } + @Test(groups = { "Functional" }) + public void testFindPositions() + { + SequenceI sq = new Sequence("test/8-13", "-ABC---DE-F--"); + + /* + * invalid inputs + */ + assertNull(sq.findPositions(6, 5)); + assertNull(sq.findPositions(0, 5)); + assertNull(sq.findPositions(-1, 5)); + + /* + * all gapped ranges + */ + assertNull(sq.findPositions(1, 1)); // 1-based columns + assertNull(sq.findPositions(5, 5)); + assertNull(sq.findPositions(5, 6)); + assertNull(sq.findPositions(5, 7)); + + /* + * all ungapped ranges + */ + assertEquals(new Range(8, 8), sq.findPositions(2, 2)); // A + assertEquals(new Range(8, 9), sq.findPositions(2, 3)); // AB + assertEquals(new Range(8, 10), sq.findPositions(2, 4)); // ABC + assertEquals(new Range(9, 10), sq.findPositions(3, 4)); // BC + + /* + * gap to ungapped range + */ + assertEquals(new Range(8, 10), sq.findPositions(1, 4)); // ABC + assertEquals(new Range(11, 12), sq.findPositions(6, 9)); // DE + + /* + * ungapped to gapped range + */ + assertEquals(new Range(10, 10), sq.findPositions(4, 5)); // C + assertEquals(new Range(9, 13), sq.findPositions(3, 11)); // BCDEF + + /* + * ungapped to ungapped enclosing gaps + */ + assertEquals(new Range(10, 11), sq.findPositions(4, 8)); // CD + assertEquals(new Range(8, 13), sq.findPositions(2, 11)); // ABCDEF + + /* + * gapped to gapped enclosing ungapped + */ + assertEquals(new Range(8, 10), sq.findPositions(1, 5)); // ABC + assertEquals(new Range(11, 12), sq.findPositions(5, 10)); // DE + assertEquals(new Range(8, 13), sq.findPositions(1, 13)); // the lot + assertEquals(new Range(8, 13), sq.findPositions(1, 99)); + } + /** * Tests for the method that returns a dataset sequence position (start..) for * an aligned column position (base 0). @@ -465,8 +520,7 @@ public class SequenceTest assertEquals("test:Pos13:Col10:startCol3:endCol10:tok0", PA.getValue(sq, "cursor").toString()); sq.sequenceChanged(); - assertEquals(12, sq.findPosition(8)); - cursor = (SequenceCursor) PA.getValue(sq, "cursor"); + assertEquals(12, sq.findPosition(8)); // E12 // sequenceChanged() invalidates cursor.lastResidueColumn cursor = (SequenceCursor) PA.getValue(sq, "cursor"); assertEquals("test:Pos12:Col9:startCol3:endCol0:tok1", @@ -505,6 +559,13 @@ public class SequenceTest assertEquals(6, sq.getEnd()); assertNull(PA.getValue(sq, "datasetSequence")); + sq = new Sequence("test", "ABCDE"); + sq.deleteChars(0, 3); + assertEquals("DE", sq.getSequenceAsString()); + assertEquals(4, sq.getStart()); + assertEquals(5, sq.getEnd()); + assertNull(PA.getValue(sq, "datasetSequence")); + /* * delete at end */ @@ -514,6 +575,21 @@ public class SequenceTest assertEquals(1, sq.getStart()); assertEquals(4, sq.getEnd()); assertNull(PA.getValue(sq, "datasetSequence")); + + /* + * delete more positions than there are + */ + sq = new Sequence("test/8-11", "ABCD"); + sq.deleteChars(0, 99); + assertEquals("", sq.getSequenceAsString()); + assertEquals(12, sq.getStart()); // = findPosition(99) ?!? + assertEquals(11, sq.getEnd()); + + sq = new Sequence("test/8-11", "----"); + sq.deleteChars(0, 99); // ArrayIndexOutOfBoundsException <= 2.10.2 + assertEquals("", sq.getSequenceAsString()); + assertEquals(8, sq.getStart()); + assertEquals(11, sq.getEnd()); } @Test(groups = { "Functional" }) @@ -1614,6 +1690,10 @@ public class SequenceTest // cursor should now be at [D 6] cursor = (SequenceCursor) PA.getValue(sq, "cursor"); assertEquals(new SequenceCursor(sq, 11, 6, ++token), cursor); + assertEquals(0, cursor.lastColumnPosition); // not yet found + assertEquals(13, sq.findPosition(8)); // E13 + cursor = (SequenceCursor) PA.getValue(sq, "cursor"); + assertEquals(9, cursor.lastColumnPosition); // found /* * deleteChars should invalidate the cached cursor @@ -1693,61 +1773,6 @@ public class SequenceTest } @Test(groups = { "Functional" }) - public void testFindPositions() - { - SequenceI sq = new Sequence("test/8-13", "-ABC---DE-F--"); - - /* - * invalid inputs - */ - assertNull(sq.findPositions(6, 5)); - assertNull(sq.findPositions(0, 5)); - assertNull(sq.findPositions(-1, 5)); - - /* - * all gapped ranges - */ - assertNull(sq.findPositions(1, 1)); // 1-based columns - assertNull(sq.findPositions(5, 5)); - assertNull(sq.findPositions(5, 6)); - assertNull(sq.findPositions(5, 7)); - - /* - * all ungapped ranges - */ - assertEquals(new Range(8, 8), sq.findPositions(2, 2)); // A - assertEquals(new Range(8, 9), sq.findPositions(2, 3)); // AB - assertEquals(new Range(8, 10), sq.findPositions(2, 4)); // ABC - assertEquals(new Range(9, 10), sq.findPositions(3, 4)); // BC - - /* - * gap to ungapped range - */ - assertEquals(new Range(8, 10), sq.findPositions(1, 4)); // ABC - assertEquals(new Range(11, 12), sq.findPositions(6, 9)); // DE - - /* - * ungapped to gapped range - */ - assertEquals(new Range(10, 10), sq.findPositions(4, 5)); // C - assertEquals(new Range(9, 13), sq.findPositions(3, 11)); // BCDEF - - /* - * ungapped to ungapped enclosing gaps - */ - assertEquals(new Range(10, 11), sq.findPositions(4, 8)); // CD - assertEquals(new Range(8, 13), sq.findPositions(2, 11)); // ABCDEF - - /* - * gapped to gapped enclosing ungapped - */ - assertEquals(new Range(8, 10), sq.findPositions(1, 5)); // ABC - assertEquals(new Range(11, 12), sq.findPositions(5, 10)); // DE - assertEquals(new Range(8, 13), sq.findPositions(1, 13)); // the lot - assertEquals(new Range(8, 13), sq.findPositions(1, 99)); - } - - @Test(groups = { "Functional" }) public void testGapBitset() { SequenceI sq = new Sequence("test/8-13", "-ABC---DE-F--"); diff --git a/test/jalview/datamodel/features/FeatureStoreTest.java b/test/jalview/datamodel/features/FeatureStoreTest.java index db21c2f..6e7dd02 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,6 +827,32 @@ 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); } @Test(groups = "Functional") diff --git a/test/jalview/datamodel/features/SequenceFeaturesTest.java b/test/jalview/datamodel/features/SequenceFeaturesTest.java index 32987b0..29e76bb 100644 --- a/test/jalview/datamodel/features/SequenceFeaturesTest.java +++ b/test/jalview/datamodel/features/SequenceFeaturesTest.java @@ -1161,7 +1161,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); @@ -1179,7 +1179,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(); @@ -1208,7 +1208,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); @@ -1218,6 +1218,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") @@ -1232,4 +1261,18 @@ public class SequenceFeaturesTest assertTrue(store.isOntologyTerm("junk", new String[] {})); assertTrue(store.isOntologyTerm("junk", (String[]) null)); } + + @Test(groups = "Functional") + public void testDeleteAll() + { + SequenceFeaturesI store = new SequenceFeatures(); + assertFalse(store.hasFeatures()); + store.deleteAll(); + assertFalse(store.hasFeatures()); + store.add(new SequenceFeature("Cath", "Desc", 12, 20, 0f, "Group")); + store.add(new SequenceFeature("Pfam", "Desc", 6, 12, 2f, "Group2")); + assertTrue(store.hasFeatures()); + store.deleteAll(); + assertFalse(store.hasFeatures()); + } }