From: Jim Procter Date: Wed, 16 Jan 2019 11:38:15 +0000 (+0000) Subject: Merge branch 'features/pca_jaxb_datasetrefs_JAL-3171_JAL-3063_JAL-1767' into develop X-Git-Tag: Release_2_11_1_0~78 X-Git-Url: http://source.jalview.org/gitweb/?p=jalview.git;a=commitdiff_plain;h=aab74f8748bf49bf6b17ba82f0bc5644d4dc2853;hp=091704be1e1e54b7990e8e6a10f7c0fd12d33416 Merge branch 'features/pca_jaxb_datasetrefs_JAL-3171_JAL-3063_JAL-1767' into develop --- diff --git a/.gitignore b/.gitignore index 86637df..d355a80 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ TESTNG /jalviewApplet.jar /benchmarking/lib *.class +/site diff --git a/help/html/features/featuresFormat.html b/help/html/features/featuresFormat.html index fd6b99f..4d13dcd 100755 --- a/help/html/features/featuresFormat.html +++ b/help/html/features/featuresFormat.html @@ -60,7 +60,10 @@ contains tab separated text fields. No comments are allowed.

-

The first set of lines contain type definitions: +

+ Feature Colours +

+

The first set of lines contain feature type definitions and their colours: 

 Feature label	Feature Colour
 
@@ -72,21 +75,37 @@
diff --git a/help/html/releases.html b/help/html/releases.html index 3eaf234..0fba08a 100755 --- a/help/html/releases.html +++ b/help/html/releases.html @@ -71,25 +71,64 @@ li:before {
2.11.0
- 8/09/2018 + 29/01/2019
-
- -
    -
  • - -
    • -
-
-
- -
    -
  • - DAS sequence retrieval and annotation capabilities removed from the Jalview Desktop -
    • -
-
+
+ Deprecations +
    +
  • + DAS sequence retrieval and annotation + capabilities removed from the Jalview Desktop +
    • +
+ Release Processes +
    +
  • Atlassian Bamboo continuous integration server for unattended Test Suite execution
    • +
  • Memory test suite to detect leaks in common operations
    • +
+
+
+ +
    +
  • + Jalview hangs when closing windows + or the overview updates with large alignments. +
    • +
  • + Tree and PCA calculation fails for selected + region if columns were selected by dragging right-to-left + and the mouse moved to the left of the first column. +
    • +
  • + Error message for trying to load in invalid + URLs doesn't tell users the invalid URL +
    • +
+ Editing +
    +
  • + Start and End should be updated when + sequence data at beginning or end of alignment added/removed + via 'Edit' sequence +
    • +
  • + Delete/Cut selection doesn't relocate + sequence features correctly when start of sequence is + removed (Known defect since 2.10) +
    • +
  • + +
    • +
+ New Known Defects +
    +
  • + Nonpositional features lose feature group + on export as jalview features file +
    • +
+
diff --git a/resources/lang/Messages.properties b/resources/lang/Messages.properties index 449c9b6..43e055d 100644 --- a/resources/lang/Messages.properties +++ b/resources/lang/Messages.properties @@ -415,7 +415,7 @@ label.input_alignment_from_url = Input Alignment From URL label.input_alignment = Input Alignment label.couldnt_import_as_vamsas_session = Couldn't import {0} as a new vamsas session. label.vamsas_document_import_failed = Vamsas Document Import Failed -label.couldnt_locate = Couldn't locate {0} +label.couldnt_locate = Could not locate {0} label.url_not_found = URL not found label.new_sequence_url_link = New sequence URL link label.cannot_edit_annotations_in_wrapped_view = Cannot edit annotations in wrapped view @@ -1282,7 +1282,6 @@ label.SEQUENCE_ID_for_DB_ACCESSION1 = Please review your URL links in the 'Conne label.SEQUENCE_ID_for_DB_ACCESSION2 = URL links using '$SEQUENCE_ID$' for DB accessions now use '$DB_ACCESSION$'. label.do_not_display_again = Do not display this message again exception.url_cannot_have_duplicate_id = {0} cannot be used as a label for more than one line -label.filter = Filter text: action.customfilter = Custom only action.showall = Show All label.insert = Insert: @@ -1350,7 +1349,6 @@ label.colour_by_text = Colour by text label.graduated_colour = Graduated Colour label.by_text_of = By text of label.by_range_of = By range of -label.filters_tooltip = Click to set or amend filters label.or = Or label.and = And label.sequence_feature_colours = Sequence Feature Colours @@ -1361,3 +1359,5 @@ label.most_bound_molecules = Most Bound Molecules label.most_polymer_residues = Most Polymer Residues label.cached_structures = Cached Structures label.free_text_search = Free Text Search +label.configuration = Configuration +label.configure_feature_tooltip = Click to configure variable colour or filters diff --git a/resources/lang/Messages_es.properties b/resources/lang/Messages_es.properties index 3c82386..5aed0cb 100644 --- a/resources/lang/Messages_es.properties +++ b/resources/lang/Messages_es.properties @@ -1283,7 +1283,6 @@ label.SEQUENCE_ID_for_DB_ACCESSION1 = Por favor, revise sus URLs en la pesta label.SEQUENCE_ID_for_DB_ACCESSION2 = URL enlaza usando '$SEQUENCE_ID$' para accesiones DB ahora usar '$DB_ACCESSION$'. label.do_not_display_again = No mostrar este mensaje de nuevo exception.url_cannot_have_duplicate_id = {0} no puede ser usada como etiqueta en más de un enlace -label.filter = Filtrar texto: action.customfilter = Sólo personalizado action.showall = Mostrar todo label.insert = Insertar: @@ -1351,7 +1350,6 @@ label.colour_by_text = Colorear por texto label.graduated_colour = Color graduado label.by_text_of = Por texto de label.by_range_of = Por rango de -label.filters_tooltip = Haga clic para configurar o modificar los filtros label.or = O label.and = Y label.sequence_feature_colours = Colores de características de las secuencias @@ -1362,3 +1360,5 @@ label.most_bound_molecules = M label.most_polymer_residues = Más Residuos de Polímeros label.cached_structures = Estructuras en Caché label.free_text_search = Búsqueda de texto libre +label.configuration = Configuración +label.configure_feature_tooltip = Haga clic para configurar el color o los filtros diff --git a/src/jalview/api/FeatureRenderer.java b/src/jalview/api/FeatureRenderer.java index cf3c8da..868f196 100644 --- a/src/jalview/api/FeatureRenderer.java +++ b/src/jalview/api/FeatureRenderer.java @@ -255,11 +255,11 @@ public interface FeatureRenderer *

* Returns null if *

+ * This method does not check feature type visibility. * * @param feature * @return diff --git a/src/jalview/appletgui/AppletJmolBinding.java b/src/jalview/appletgui/AppletJmolBinding.java index 2f61b24..e5767b6 100644 --- a/src/jalview/appletgui/AppletJmolBinding.java +++ b/src/jalview/appletgui/AppletJmolBinding.java @@ -24,7 +24,6 @@ import jalview.api.AlignmentViewPanel; import jalview.datamodel.PDBEntry; import jalview.datamodel.SequenceI; import jalview.ext.jmol.JalviewJmolBinding; -import jalview.gui.IProgressIndicator; import jalview.io.DataSourceType; import jalview.structure.StructureSelectionManager; @@ -174,21 +173,12 @@ class AppletJmolBinding extends JalviewJmolBinding @Override public int[] resizeInnerPanel(String data) { - // TODO Auto-generated method stub return null; } @Override public Map getJSpecViewProperty(String arg0) { - // TODO Auto-generated method stub - return null; - } - - @Override - protected IProgressIndicator getIProgressIndicator() - { - // no progress indicators on the applet return null; } } diff --git a/src/jalview/appletgui/ExtJmol.java b/src/jalview/appletgui/ExtJmol.java index 89228d5..b0d3f7a 100644 --- a/src/jalview/appletgui/ExtJmol.java +++ b/src/jalview/appletgui/ExtJmol.java @@ -26,7 +26,6 @@ import jalview.api.SequenceRenderer; import jalview.datamodel.PDBEntry; import jalview.datamodel.SequenceI; import jalview.ext.jmol.JalviewJmolBinding; -import jalview.gui.IProgressIndicator; import jalview.io.DataSourceType; import java.awt.Container; @@ -66,18 +65,8 @@ public class ExtJmol extends JalviewJmolBinding } @Override - protected IProgressIndicator getIProgressIndicator() - { - // no progress indicators on applet (could access javascript for this) - return null; - } - - @Override public void updateColours(Object source) { - - // TODO Auto-generated method stub - } @Override @@ -190,7 +179,6 @@ public class ExtJmol extends JalviewJmolBinding protected JmolAppConsoleInterface createJmolConsole( Container consolePanel, String buttonsToShow) { - // TODO Auto-generated method stub return null; } @@ -205,14 +193,11 @@ public class ExtJmol extends JalviewJmolBinding @Override public void releaseReferences(Object svl) { - // TODO Auto-generated method stub - } @Override public Map getJSpecViewProperty(String arg0) { - // TODO Auto-generated method stub return null; } diff --git a/src/jalview/appletgui/OverviewCanvas.java b/src/jalview/appletgui/OverviewCanvas.java index e99c021..07f5919 100644 --- a/src/jalview/appletgui/OverviewCanvas.java +++ b/src/jalview/appletgui/OverviewCanvas.java @@ -162,4 +162,12 @@ public class OverviewCanvas extends Component } } + /** + * Nulls references to protect against potential memory leaks + */ + void dispose() + { + od = null; + } + } diff --git a/src/jalview/appletgui/OverviewPanel.java b/src/jalview/appletgui/OverviewPanel.java index 3bbbe95..5081509 100755 --- a/src/jalview/appletgui/OverviewPanel.java +++ b/src/jalview/appletgui/OverviewPanel.java @@ -334,6 +334,10 @@ public class OverviewPanel extends Panel implements Runnable, } finally { av = null; + if (oviewCanvas != null) + { + oviewCanvas.dispose(); + } oviewCanvas = null; ap = null; od = null; 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/SequenceGroup.java b/src/jalview/datamodel/SequenceGroup.java index 944f263..fc8ac49 100755 --- a/src/jalview/datamodel/SequenceGroup.java +++ b/src/jalview/datamodel/SequenceGroup.java @@ -102,11 +102,15 @@ public class SequenceGroup implements AnnotatedCollectionI */ public ResidueShaderI cs; - // start column (base 0) - int startRes = 0; + /** + * start column (base 0) + */ + private int startRes = 0; - // end column (base 0) - int endRes = 0; + /** + * end column (base 0) + */ + private int endRes = 0; public Color outlineColour = Color.black; @@ -209,6 +213,7 @@ public class SequenceGroup implements AnnotatedCollectionI displayBoxes = seqsel.displayBoxes; displayText = seqsel.displayText; colourText = seqsel.colourText; + startRes = seqsel.startRes; endRes = seqsel.endRes; cs = new ResidueShader((ResidueShader) seqsel.cs); @@ -752,13 +757,16 @@ public class SequenceGroup implements AnnotatedCollectionI /** * Set the first column selected by this group. Runs from 0<=i 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/ext/jmol/JalviewJmolBinding.java b/src/jalview/ext/jmol/JalviewJmolBinding.java index 8832278..a5b1110 100644 --- a/src/jalview/ext/jmol/JalviewJmolBinding.java +++ b/src/jalview/ext/jmol/JalviewJmolBinding.java @@ -1254,7 +1254,10 @@ public abstract class JalviewJmolBinding extends AAStructureBindingModel return chainNames; } - protected abstract IProgressIndicator getIProgressIndicator(); + protected IProgressIndicator getIProgressIndicator() + { + return null; + } public void notifyNewPickingModeMeasurement(int iatom, String strMeasure) { diff --git a/src/jalview/gui/FeatureSettings.java b/src/jalview/gui/FeatureSettings.java index 90d7c35..dbe3317 100644 --- a/src/jalview/gui/FeatureSettings.java +++ b/src/jalview/gui/FeatureSettings.java @@ -79,7 +79,6 @@ import javax.swing.BorderFactory; import javax.swing.Icon; import javax.swing.JButton; import javax.swing.JCheckBox; -import javax.swing.JCheckBoxMenuItem; import javax.swing.JColorChooser; import javax.swing.JDialog; import javax.swing.JInternalFrame; @@ -219,7 +218,8 @@ public class FeatureSettings extends JPanel FeatureMatcherSet o = (FeatureMatcherSet) table.getValueAt(row, column); tip = o.isEmpty() - ? MessageManager.getString("label.filters_tooltip") + ? MessageManager + .getString("label.configure_feature_tooltip") : o.toString(); break; default: @@ -415,69 +415,6 @@ public class FeatureSettings extends JPanel }); men.add(dens); - /* - * variable colour options include colour by label, by score, - * by selected attribute text, or attribute value - */ - final JCheckBoxMenuItem mxcol = new JCheckBoxMenuItem( - MessageManager.getString("label.variable_colour")); - mxcol.setSelected(!featureColour.isSimpleColour()); - men.add(mxcol); - mxcol.addActionListener(new ActionListener() - { - JColorChooser colorChooser; - - @Override - public void actionPerformed(ActionEvent e) - { - if (e.getSource() == mxcol) - { - if (featureColour.isSimpleColour()) - { - FeatureTypeSettings fc = new FeatureTypeSettings(me.fr, type); - fc.addActionListener(this); - } - else - { - // bring up simple color chooser - colorChooser = new JColorChooser(); - String title = MessageManager - .getString("label.select_colour"); - JDialog dialog = JColorChooser.createDialog(me, - title, true, // modal - colorChooser, this, // OK button handler - null); // no CANCEL button handler - colorChooser.setColor(featureColour.getMaxColour()); - dialog.setVisible(true); - } - } - else - { - if (e.getSource() instanceof FeatureTypeSettings) - { - /* - * update after OK in feature colour dialog; the updated - * colour will have already been set in the FeatureRenderer - */ - FeatureColourI fci = fr.getFeatureColours().get(type); - table.setValueAt(fci, rowSelected, 1); - table.validate(); - } - else - { - // probably the color chooser! - table.setValueAt(new FeatureColour(colorChooser.getColor()), - rowSelected, 1); - table.validate(); - me.updateFeatureRenderer( - ((FeatureTableModel) table.getModel()).getData(), - false); - } - } - } - - }); - JMenuItem selCols = new JMenuItem( MessageManager.getString("label.select_columns_containing")); selCols.addActionListener(new ActionListener() @@ -1354,7 +1291,7 @@ public class FeatureSettings extends JPanel private String[] columnNames = { MessageManager.getString("label.feature_type"), MessageManager.getString("action.colour"), - MessageManager.getString("label.filter"), + MessageManager.getString("label.configuration"), MessageManager.getString("label.show") }; private Object[][] data; @@ -1690,14 +1627,17 @@ public class FeatureSettings extends JPanel { // bring up graduated chooser. chooser = new FeatureTypeSettings(me.fr, type); - chooser.setRequestFocusEnabled(true); - chooser.requestFocus(); + /** + * @j2sNative + */ + { + chooser.setRequestFocusEnabled(true); + chooser.requestFocus(); + } chooser.addActionListener(this); - chooser.showTab(true); + // Make the renderer reappear. + fireEditingStopped(); } - // Make the renderer reappear. - fireEditingStopped(); - } else { @@ -1819,7 +1759,6 @@ public class FeatureSettings extends JPanel chooser.getWidth(), chooser.getHeight()); chooser.validate(); } - chooser.showTab(false); fireEditingStopped(); } else if (e.getSource() instanceof Component) diff --git a/src/jalview/gui/FeatureTypeSettings.java b/src/jalview/gui/FeatureTypeSettings.java index e13f6ee..58bbbe8 100644 --- a/src/jalview/gui/FeatureTypeSettings.java +++ b/src/jalview/gui/FeatureTypeSettings.java @@ -62,7 +62,6 @@ import javax.swing.JLabel; import javax.swing.JPanel; import javax.swing.JRadioButton; import javax.swing.JSlider; -import javax.swing.JTabbedPane; import javax.swing.JTextField; import javax.swing.SwingConstants; import javax.swing.border.LineBorder; @@ -107,7 +106,8 @@ public class FeatureTypeSettings extends JalviewDialog /* * FeatureRenderer holds colour scheme and filters for feature types */ - private final FeatureRenderer fr; // todo refactor to allow interface type here + private final FeatureRenderer fr; // todo refactor to allow interface type + // here /* * the view panel to update when settings change @@ -155,7 +155,12 @@ public class FeatureTypeSettings extends JalviewDialog private JRadioButton graduatedColour = new JRadioButton(); - private JPanel singleColour = new JPanel(); + /** + * colours and filters are shown in tabbed view or single content pane + */ + JPanel coloursPanel, filtersPanel; + + JPanel singleColour = new JPanel(); private JPanel minColour = new JPanel(); @@ -199,13 +204,8 @@ public class FeatureTypeSettings extends JalviewDialog */ private List filters; - // set white normally, black to debug layout - private Color debugBorderColour = Color.white; - private JPanel chooseFiltersPanel; - private JTabbedPane tabbedPane; - /** * Constructor * @@ -214,28 +214,14 @@ public class FeatureTypeSettings extends JalviewDialog */ public FeatureTypeSettings(FeatureRenderer frender, String theType) { - this(frender, false, theType); - } - - /** - * Constructor, with option to make a blocking dialog (has to complete in the - * AWT event queue thread). Currently this option is always set to false. - * - * @param frender - * @param blocking - * @param theType - */ - FeatureTypeSettings(FeatureRenderer frender, boolean blocking, - String theType) - { this.fr = frender; this.featureType = theType; ap = fr.ap; originalFilter = fr.getFeatureFilter(theType); originalColour = fr.getFeatureColours().get(theType); - + adjusting = true; - + try { initialise(); @@ -244,20 +230,19 @@ public class FeatureTypeSettings extends JalviewDialog ex.printStackTrace(); return; } - + updateColoursTab(); - + updateFiltersTab(); - + adjusting = false; - + colourChanged(false); - + String title = MessageManager .formatMessage("label.display_settings_for", new String[] { theType }); - initDialogFrame(this, true, blocking, title, 600, 360); - + initDialogFrame(this, true, false, title, 580, 500); waitForInput(); } @@ -280,9 +265,9 @@ public class FeatureTypeSettings extends JalviewDialog */ if (fc.isSimpleColour()) { - simpleColour.setSelected(true); singleColour.setBackground(fc.getColour()); singleColour.setForeground(fc.getColour()); + simpleColour.setSelected(true); } /* @@ -380,7 +365,7 @@ public class FeatureTypeSettings extends JalviewDialog : BELOW_THRESHOLD_OPTION); slider.setEnabled(true); slider.setValue((int) (fc.getThreshold() * scaleFactor)); - thresholdValue.setText(String.valueOf(getRoundedSliderValue())); + thresholdValue.setText(String.valueOf(fc.getThreshold())); thresholdValue.setEnabled(true); thresholdIsMin.setEnabled(true); } @@ -403,8 +388,6 @@ public class FeatureTypeSettings extends JalviewDialog private void initialise() { this.setLayout(new BorderLayout()); - tabbedPane = new JTabbedPane(); - this.add(tabbedPane, BorderLayout.CENTER); /* * an ActionListener that applies colour changes @@ -419,18 +402,16 @@ public class FeatureTypeSettings extends JalviewDialog }; /* - * first tab: colour options + * first panel/tab: colour options */ JPanel coloursPanel = initialiseColoursPanel(); - tabbedPane.addTab(MessageManager.getString("action.colour"), - coloursPanel); + this.add(coloursPanel, BorderLayout.NORTH); /* - * second tab: filter options + * second panel/tab: filter options */ JPanel filtersPanel = initialiseFiltersPanel(); - tabbedPane.addTab(MessageManager.getString("label.filters"), - filtersPanel); + this.add(filtersPanel, BorderLayout.CENTER); JPanel okCancelPanel = initialiseOkCancelPanel(); @@ -581,12 +562,11 @@ public class FeatureTypeSettings extends JalviewDialog maxColour.setBorder(new LineBorder(Color.black)); /* - * default max colour to current colour (if a plain colour), - * or to Black if colour by label; make min colour a pale - * version of max colour + * default max colour to last plain colour; + * make min colour a pale version of max colour */ FeatureColourI fc = fr.getFeatureColours().get(featureType); - Color bg = fc.isSimpleColour() ? fc.getColour() : Color.BLACK; + Color bg = fc.getColour() == null ? Color.BLACK : fc.getColour(); maxColour.setBackground(bg); minColour.setBackground(ColorUtils.bleachColour(bg, 0.9f)); @@ -671,6 +651,7 @@ public class FeatureTypeSettings extends JalviewDialog { thresholdValue .setText(String.valueOf(slider.getValue() / scaleFactor)); + thresholdValue.setBackground(Color.white); // to reset red for invalid sliderValueChanged(); } } @@ -736,15 +717,16 @@ public class FeatureTypeSettings extends JalviewDialog private JPanel initialiseColoursPanel() { JPanel colourByPanel = new JPanel(); + colourByPanel.setBackground(Color.white); colourByPanel.setLayout(new BoxLayout(colourByPanel, BoxLayout.Y_AXIS)); + JvSwingUtils.createTitledBorder(colourByPanel, + MessageManager.getString("action.colour"), true); /* * simple colour radio button and colour picker */ JPanel simpleColourPanel = new JPanel(new FlowLayout(FlowLayout.LEFT)); simpleColourPanel.setBackground(Color.white); - JvSwingUtils.createTitledBorder(simpleColourPanel, - MessageManager.getString("label.simple"), true); colourByPanel.add(simpleColourPanel); simpleColour = new JRadioButton( @@ -757,15 +739,24 @@ public class FeatureTypeSettings extends JalviewDialog { if (simpleColour.isSelected() && !adjusting) { - showColourChooser(singleColour, "label.select_colour"); + colourChanged(true); } } - }); - + singleColour.setFont(JvSwingUtils.getLabelFont()); singleColour.setBorder(BorderFactory.createLineBorder(Color.black)); singleColour.setPreferredSize(new Dimension(40, 20)); + if (originalColour.isGraduatedColour()) + { + singleColour.setBackground(originalColour.getMaxColour()); + singleColour.setForeground(originalColour.getMaxColour()); + } + else + { + singleColour.setBackground(originalColour.getColour()); + singleColour.setForeground(originalColour.getColour()); + } singleColour.addMouseListener(new MouseAdapter() { @Override @@ -849,9 +840,9 @@ public class FeatureTypeSettings extends JalviewDialog } /** - * Constructs and sets the selected colour options as the colour for the feature - * type, and repaints the alignment, and optionally the Overview and/or - * structure viewer if open + * Constructs and sets the selected colour options as the colour for the + * feature type, and repaints the alignment, and optionally the Overview + * and/or structure viewer if open * * @param updateStructsAndOverview */ @@ -902,8 +893,8 @@ public class FeatureTypeSettings extends JalviewDialog */ if (byCategory.isSelected()) { - Color c = this.getBackground(); - FeatureColourI fc = new FeatureColour(c, c, null, 0f, 0f); + Color c = singleColour.getBackground(); + FeatureColourI fc = new FeatureColour(c); fc.setColourByLabel(true); String byWhat = (String) colourByTextCombo.getSelectedItem(); if (!LABEL_18N.equals(byWhat)) @@ -1028,21 +1019,23 @@ public class FeatureTypeSettings extends JalviewDialog { try { + /* + * set 'adjusting' flag while moving the slider, so it + * doesn't then in turn change the value (with rounding) + */ adjusting = true; float f = Float.parseFloat(thresholdValue.getText()); + f = Float.max(f, this.min); + f = Float.min(f, this.max); + thresholdValue.setText(String.valueOf(f)); slider.setValue((int) (f * scaleFactor)); threshline.value = f; thresholdValue.setBackground(Color.white); // ok - - /* - * force repaint of any Overview window or structure - */ - ap.paintAlignment(true, true); + adjusting = false; + colourChanged(true); } catch (NumberFormatException ex) { thresholdValue.setBackground(Color.red); // not ok - } finally - { adjusting = false; } } @@ -1065,8 +1058,8 @@ public class FeatureTypeSettings extends JalviewDialog /** * Converts the slider value to its absolute value by dividing by the - * scaleFactor. Rounding errors are squashed by forcing min/max of slider range - * to the actual min/max of feature score range + * scaleFactor. Rounding errors are squashed by forcing min/max of slider + * range to the actual min/max of feature score range * * @return */ @@ -1089,11 +1082,11 @@ public class FeatureTypeSettings extends JalviewDialog } /** - * A helper method to build the drop-down choice of attributes for a feature. If - * 'withRange' is true, then Score, and any attributes with a min-max range, are - * added. If 'withText' is true, Label and any known attributes are added. This - * allows 'categorical numerical' attributes e.g. codon position to be coloured - * by text. + * A helper method to build the drop-down choice of attributes for a feature. + * If 'withRange' is true, then Score, and any attributes with a min-max + * range, are added. If 'withText' is true, Label and any known attributes are + * added. This allows 'categorical numerical' attributes e.g. codon position + * to be coloured by text. *

* Where metadata is available with a description for an attribute, that is * added as a tooltip. @@ -1188,7 +1181,6 @@ public class FeatureTypeSettings extends JalviewDialog { JPanel andOrPanel = new JPanel(new FlowLayout(FlowLayout.LEFT)); andOrPanel.setBackground(Color.white); - andOrPanel.setBorder(BorderFactory.createLineBorder(debugBorderColour)); andFilters = new JRadioButton(MessageManager.getString("label.and")); orFilters = new JRadioButton(MessageManager.getString("label.or")); ActionListener actionListener = new ActionListener() @@ -1271,7 +1263,6 @@ public class FeatureTypeSettings extends JalviewDialog for (FeatureMatcherI filter : filters) { JPanel row = addFilter(filter, attNames, filterIndex); - row.setBorder(BorderFactory.createLineBorder(debugBorderColour)); chooseFiltersPanel.add(row); filterIndex++; } @@ -1283,7 +1274,8 @@ public class FeatureTypeSettings extends JalviewDialog /** * A helper method that constructs a row (panel) with one filter condition: *

    - *
  • a drop-down list of Label, Score and attribute names to choose from
    • + *
  • a drop-down list of Label, Score and attribute names to choose + * from
    • *
  • a drop-down list of conditions to choose from
    • *
  • a text field for input of a match pattern
    • *
  • optionally, a 'remove' button
    • @@ -1481,8 +1473,8 @@ public class FeatureTypeSettings extends JalviewDialog * @param selectedCondition * @param patternField */ - private void setNumericHints(String attName, - Condition selectedCondition, JTextField patternField) + private void setNumericHints(String attName, Condition selectedCondition, + JTextField patternField) { patternField.setToolTipText(""); @@ -1516,11 +1508,11 @@ public class FeatureTypeSettings extends JalviewDialog } /** - * Populates the drop-down list of comparison conditions for the given attribute - * name. The conditions added depend on the datatype of the attribute values. - * The supplied condition is set as the selected item in the list, provided it - * is in the list. If the pattern is now invalid (non-numeric pattern for a - * numeric condition), it is cleared. + * Populates the drop-down list of comparison conditions for the given + * attribute name. The conditions added depend on the datatype of the + * attribute values. The supplied condition is set as the selected item in the + * list, provided it is in the list. If the pattern is now invalid + * (non-numeric pattern for a numeric condition), it is cleared. * * @param attName * @param cond @@ -1599,12 +1591,12 @@ public class FeatureTypeSettings extends JalviewDialog } /** - * Answers true unless a numeric condition has been selected with a non-numeric - * value. Sets the value field to RED with a tooltip if in error. + * Answers true unless a numeric condition has been selected with a + * non-numeric value. Sets the value field to RED with a tooltip if in error. *

    - * If the pattern is expected but is empty, this method returns false, but does - * not mark the field as invalid. This supports selecting an attribute for a new - * condition before a match pattern has been entered. + * If the pattern is expected but is empty, this method returns false, but + * does not mark the field as invalid. This supports selecting an attribute + * for a new condition before a match pattern has been entered. * * @param value * @param condCombo @@ -1650,14 +1642,15 @@ public class FeatureTypeSettings extends JalviewDialog /** * Constructs a filter condition from the given input fields, and replaces the - * condition at filterIndex with the new one. Does nothing if the pattern field - * is blank (unless the match condition is one that doesn't require a pattern, - * e.g. 'Is present'). Answers true if the filter was updated, else false. + * condition at filterIndex with the new one. Does nothing if the pattern + * field is blank (unless the match condition is one that doesn't require a + * pattern, e.g. 'Is present'). Answers true if the filter was updated, else + * false. *

    * This method may update the tooltip on the filter value field to show the - * value range, if a numeric condition is selected. This ensures the tooltip is - * updated when a numeric valued attribute is chosen on the last 'add a filter' - * row. + * value range, if a numeric condition is selected. This ensures the tooltip + * is updated when a numeric valued attribute is chosen on the last 'add a + * filter' row. * * @param attCombo * @param condCombo @@ -1705,17 +1698,6 @@ public class FeatureTypeSettings extends JalviewDialog } /** - * Makes the dialog visible, at the Feature Colour tab or at the Filters tab - * - * @param coloursTab - */ - public void showTab(boolean coloursTab) - { - setVisible(true); - tabbedPane.setSelectedIndex(coloursTab ? 0 : 1); - } - - /** * Action on any change to feature filtering, namely *

      *
    • change of selected attribute
      • @@ -1723,8 +1705,8 @@ public class FeatureTypeSettings extends JalviewDialog *
    • change of match pattern
      • *
    • removal of a condition
      • *
    - * The inputs are parsed into a combined filter and this is set for the feature - * type, and the alignment redrawn. + * The inputs are parsed into a combined filter and this is set for the + * feature type, and the alignment redrawn. */ protected void filtersChanged() { diff --git a/src/jalview/gui/OverviewCanvas.java b/src/jalview/gui/OverviewCanvas.java index cc361a5..89088b8d 100644 --- a/src/jalview/gui/OverviewCanvas.java +++ b/src/jalview/gui/OverviewCanvas.java @@ -257,6 +257,7 @@ public class OverviewCanvas extends JComponent public void dispose() { dispose = true; + od = null; synchronized (this) { restart = true; diff --git a/src/jalview/gui/ScalePanel.java b/src/jalview/gui/ScalePanel.java index e6bba02..6abee08 100755 --- a/src/jalview/gui/ScalePanel.java +++ b/src/jalview/gui/ScalePanel.java @@ -276,7 +276,9 @@ public class ScalePanel extends JPanel { mouseDragging = false; - int res = (evt.getX() / av.getCharWidth()) + int xCords = Math.max(0, evt.getX()); // prevent negative X coordinates + + int res = (xCords / av.getCharWidth()) + av.getRanges().getStartRes(); if (av.hasHiddenColumns()) diff --git a/src/jalview/schemes/FeatureColour.java b/src/jalview/schemes/FeatureColour.java index 7d14662..51e7645 100644 --- a/src/jalview/schemes/FeatureColour.java +++ b/src/jalview/schemes/FeatureColour.java @@ -321,8 +321,8 @@ public class FeatureColour implements FeatureColourI } catch (Exception e) { throw new IllegalArgumentException( - "Couldn't parse the minimum value for graduated colour (" - + descriptor + ")"); + "Couldn't parse the minimum value for graduated colour ('" + + minval + "')"); } try { 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()); + } } diff --git a/test/jalview/ext/htsjdk/TestHtsContigDb.java b/test/jalview/ext/htsjdk/TestHtsContigDb.java index 28c5cf0..69634a9 100644 --- a/test/jalview/ext/htsjdk/TestHtsContigDb.java +++ b/test/jalview/ext/htsjdk/TestHtsContigDb.java @@ -44,12 +44,12 @@ public class TestHtsContigDb @Test(groups = "Functional") public final void testGetSequenceProxy() throws Exception { - String pathname = "test/jalview/ext/htsjdk/pgmb.fasta"; + String pathname = "test/jalview/ext/htsjdk/pgmB.fasta"; HtsContigDb db = new HtsContigDb("ADB", new File(pathname)); - + assertTrue(db.isValid()); assertTrue(db.isIndexed()); // htsjdk opens the .fai file - + SequenceI sq = db.getSequenceProxy("Deminut"); assertNotNull(sq); assertEquals(sq.getLength(), 606); @@ -60,7 +60,7 @@ public class TestHtsContigDb sq = db.getSequenceProxy("PPL_06716"); assertNotNull(sq); assertEquals(sq.getLength(), 602); - + // dict = db.getDictionary(f, truncate)) } @@ -73,7 +73,7 @@ public class TestHtsContigDb expectedExceptions = java.lang.IllegalArgumentException.class) public final void testGetSequenceProxy_indexed() { - String pathname = "test/jalview/ext/htsjdk/pgmb.fasta.fai"; + String pathname = "test/jalview/ext/htsjdk/pgmB.fasta.fai"; new HtsContigDb("ADB", new File(pathname)); fail("Expected exception opening .fai file"); } @@ -92,17 +92,19 @@ public class TestHtsContigDb @Test(groups = "Functional") public void testCreateFastaSequenceIndex() throws IOException { - File fasta = new File("test/jalview/ext/htsjdk/pgmb.fasta"); - + File fasta = new File("test/jalview/ext/htsjdk/pgmB.fasta"); + /* * create .fai with no overwrite fails if it exists */ - try { + try + { HtsContigDb.createFastaSequenceIndex(fasta.toPath(), false); fail("Expected exception"); } catch (IOException e) { - // expected + // we expect an IO Exception because the pgmB.fasta.fai exists, since it + // was checked it in. } /* diff --git a/test/jalview/ext/jmol/JmolParserTest.java b/test/jalview/ext/jmol/JmolParserTest.java index f5e637c..bcad464 100644 --- a/test/jalview/ext/jmol/JmolParserTest.java +++ b/test/jalview/ext/jmol/JmolParserTest.java @@ -62,9 +62,9 @@ public class JmolParserTest * 1GAQ has been reduced to alpha carbons only * 1QCF is the full PDB file including headers, HETATM etc */ - String[] testFile = new String[] { "./examples/1GAQ.txt", + String[] testFile = new String[] { "./examples/1gaq.txt", "./test/jalview/ext/jmol/1xyz.pdb", - "./test/jalview/ext/jmol/1qcf.pdb" }; + "./test/jalview/ext/jmol/1QCF.pdb" }; //@formatter:off // a modified and very cut-down extract of 4UJ4 @@ -130,17 +130,17 @@ public class JmolParserTest JmolParser jtest = new JmolParser(pdbStr, DataSourceType.FILE); Vector seqs = jtest.getSeqs(), mcseqs = mctest.getSeqs(); - assertTrue( - "No sequences extracted from testfile\n" - + (jtest.hasWarningMessage() ? jtest.getWarningMessage() - : "(No warnings raised)"), seqs != null - && seqs.size() > 0); + assertTrue("No sequences extracted from testfile\n" + + (jtest.hasWarningMessage() ? jtest.getWarningMessage() + : "(No warnings raised)"), + seqs != null && seqs.size() > 0); for (SequenceI sq : seqs) { - assertEquals("JMol didn't process " + pdbStr - + " to the same sequence as MCView", - sq.getSequenceAsString(), mcseqs.remove(0) - .getSequenceAsString()); + assertEquals( + "JMol didn't process " + pdbStr + + " to the same sequence as MCView", + sq.getSequenceAsString(), + mcseqs.remove(0).getSequenceAsString()); AlignmentI al = new Alignment(new SequenceI[] { sq }); validateSecStrRows(al); } @@ -175,13 +175,15 @@ public class JmolParserTest private void checkFirstAAIsAssoc(SequenceI sq) { - assertTrue("No secondary structure assigned for protein sequence for " - + sq.getName(), + assertTrue( + "No secondary structure assigned for protein sequence for " + + sq.getName(), sq.getAnnotation() != null && sq.getAnnotation().length >= 1 && sq.getAnnotation()[0].hasIcons); assertTrue( "Secondary structure not associated for sequence " - + sq.getName(), sq.getAnnotation()[0].sequenceRef == sq); + + sq.getName(), + sq.getAnnotation()[0].sequenceRef == sq); } /** @@ -194,7 +196,8 @@ public class JmolParserTest { PDBfile mctest = new PDBfile(false, false, false, pastePDBDataWithChainBreak, DataSourceType.PASTE); - JmolParser jtest = new JmolParser(pastePDBDataWithChainBreak, DataSourceType.PASTE); + JmolParser jtest = new JmolParser(pastePDBDataWithChainBreak, + DataSourceType.PASTE); Vector seqs = jtest.getSeqs(); Vector mcseqs = mctest.getSeqs(); @@ -216,8 +219,7 @@ public class JmolParserTest { PDBfile mctest = new PDBfile(false, false, false, pdbWithAltLoc, DataSourceType.PASTE); - JmolParser jtest = new JmolParser(pdbWithAltLoc, - DataSourceType.PASTE); + JmolParser jtest = new JmolParser(pdbWithAltLoc, DataSourceType.PASTE); Vector seqs = jtest.getSeqs(); Vector mcseqs = mctest.getSeqs(); diff --git a/test/jalview/ext/rbvi/chimera/ChimeraConnect.java b/test/jalview/ext/rbvi/chimera/ChimeraConnect.java index 4d904cf..99394dc 100644 --- a/test/jalview/ext/rbvi/chimera/ChimeraConnect.java +++ b/test/jalview/ext/rbvi/chimera/ChimeraConnect.java @@ -41,7 +41,7 @@ public class ChimeraConnect JvOptionPane.setMockResponse(JvOptionPane.CANCEL_OPTION); } - @Test(groups = { "Functional" }) + @Test(groups = { "External" }) public void testLaunchAndExit() { final StructureManager structureManager = new StructureManager(true); diff --git a/test/jalview/gui/AlignViewportTest.java b/test/jalview/gui/AlignViewportTest.java index 5ed0cac..7801250 100644 --- a/test/jalview/gui/AlignViewportTest.java +++ b/test/jalview/gui/AlignViewportTest.java @@ -277,7 +277,7 @@ public class AlignViewportTest * Test for JAL-1306 - conservation thread should run even when only Quality * (and not Conservation) is enabled in Preferences */ - @Test(groups = { "Functional" }) + @Test(groups = { "Functional" }, timeOut=2000) public void testUpdateConservation_qualityOnly() { Cache.applicationProperties.setProperty("SHOW_ANNOTATIONS", @@ -292,7 +292,24 @@ public class AlignViewportTest Boolean.FALSE.toString()); AlignFrame af = new FileLoader().LoadFileWaitTillLoaded( "examples/uniref50.fa", DataSourceType.FILE); - AlignmentAnnotation[] anns = af.viewport.getAlignment() + + /* + * wait for Conservation thread to complete + */ + AlignViewport viewport = af.getViewport(); + synchronized (this) + { + while (viewport.getAlignmentConservationAnnotation() != null) + { + try + { + wait(50); + } catch (InterruptedException e) + { + } + } + } + AlignmentAnnotation[] anns = viewport.getAlignment() .getAlignmentAnnotation(); assertNotNull("No annotations found", anns); assertEquals("More than one annotation found", 1, anns.length); diff --git a/test/jalview/gui/AlignmentPanelTest.java b/test/jalview/gui/AlignmentPanelTest.java index 2819dbf..e84b87a 100644 --- a/test/jalview/gui/AlignmentPanelTest.java +++ b/test/jalview/gui/AlignmentPanelTest.java @@ -222,7 +222,7 @@ public class AlignmentPanelTest /** * Test that update layout reverts to original (unwrapped) values for endRes - * and endSeq when switching from wrapped to unwrapped mode (JAL-2739) + * when switching from wrapped back to unwrapped mode (JAL-2739) */ @Test(groups = "Functional") public void TestUpdateLayout_endRes() @@ -235,14 +235,14 @@ public class AlignmentPanelTest af.alignPanel.getAlignViewport().setWrapAlignment(true); af.alignPanel.updateLayout(); - // endRes changes + // endRes has changed assertNotEquals(ranges.getEndRes(), endres); // unwrap af.alignPanel.getAlignViewport().setWrapAlignment(false); af.alignPanel.updateLayout(); - // endRes and endSeq back to original values + // endRes back to original value assertEquals(ranges.getEndRes(), endres); } diff --git a/test/jalview/gui/FreeUpMemoryTest.java b/test/jalview/gui/FreeUpMemoryTest.java index e93bfac..9b21274 100644 --- a/test/jalview/gui/FreeUpMemoryTest.java +++ b/test/jalview/gui/FreeUpMemoryTest.java @@ -1,6 +1,7 @@ package jalview.gui; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; import jalview.analysis.AlignmentGenerator; @@ -11,6 +12,7 @@ import jalview.datamodel.SequenceGroup; import jalview.io.DataSourceType; import jalview.io.FileLoader; +import java.awt.event.MouseEvent; import java.io.File; import java.io.IOException; import java.io.PrintStream; @@ -18,6 +20,8 @@ import java.io.PrintStream; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; +import junit.extensions.PA; + public class FreeUpMemoryTest { private static final int ONE_MB = 1000 * 1000; @@ -30,16 +34,12 @@ public class FreeUpMemoryTest { Jalview.main(new String[] { "-nonews", "-props", "test/jalview/testProps.jvprops" }); - Cache.applicationProperties.setProperty("SHOW_ANNOTATIONS", - Boolean.TRUE.toString()); - Cache.applicationProperties.setProperty("SHOW_QUALITY", - Boolean.TRUE.toString()); - Cache.applicationProperties.setProperty("SHOW_CONSERVATION", - Boolean.TRUE.toString()); - Cache.applicationProperties.setProperty("SHOW_OCCUPANCY", - Boolean.TRUE.toString()); - Cache.applicationProperties.setProperty("SHOW_IDENTITY", - Boolean.TRUE.toString()); + String True = Boolean.TRUE.toString(); + Cache.applicationProperties.setProperty("SHOW_ANNOTATIONS", True); + Cache.applicationProperties.setProperty("SHOW_QUALITY", True); + Cache.applicationProperties.setProperty("SHOW_CONSERVATION", True); + Cache.applicationProperties.setProperty("SHOW_OCCUPANCY", True); + Cache.applicationProperties.setProperty("SHOW_IDENTITY", True); } /** @@ -84,18 +84,19 @@ public class FreeUpMemoryTest protected void checkUsedMemory(long expectedMax) { /* - * request garbage collection and wait briefly for it to run; + * request garbage collection and wait for it to run; * NB there is no guarantee when, or whether, it will do so + * wait time depends on JRE/processor, generous allowance here */ System.gc(); - waitFor(100); + waitFor(1500); /* * a second gc() call should not be necessary - but it is! * the test passes with it, and fails without it */ System.gc(); - waitFor(100); + waitFor(1500); /* * check used memory is 'reasonably low' @@ -142,6 +143,20 @@ public class FreeUpMemoryTest } /* + * open an Overview window + */ + af.overviewMenuItem_actionPerformed(null); + assertNotNull(af.alignPanel.overviewPanel); + + /* + * exercise the pop-up menu in the Overview Panel (JAL-2864) + */ + Object[] args = new Object[] { + new MouseEvent(af, 0, 0, 0, 0, 0, 1, true) }; + PA.invokeMethod(af.alignPanel.overviewPanel, + "showPopupMenu(java.awt.event.MouseEvent)", args); + + /* * set a selection group - potential memory leak if it retains * a reference to the alignment */ diff --git a/test/jalview/gui/ScalePanelTest.java b/test/jalview/gui/ScalePanelTest.java new file mode 100644 index 0000000..91b541c --- /dev/null +++ b/test/jalview/gui/ScalePanelTest.java @@ -0,0 +1,58 @@ +package jalview.gui; + +import static org.testng.Assert.assertTrue; + +import jalview.datamodel.Alignment; +import jalview.datamodel.AlignmentI; +import jalview.datamodel.Sequence; +import jalview.datamodel.SequenceGroup; +import jalview.datamodel.SequenceI; + +import java.awt.event.MouseEvent; + +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +public class ScalePanelTest +{ + @BeforeClass(alwaysRun = true) + public void setUpJvOptionPane() + { + JvOptionPane.setInteractiveMode(false); + JvOptionPane.setMockResponse(JvOptionPane.CANCEL_OPTION); + } + + @Test(groups = "Functional") + public void testPreventNegativeStartColumn() + { + SequenceI seq1 = new Sequence("Seq1", "MATRESS"); + SequenceI seq2 = new Sequence("Seq2", "MADNESS"); + AlignmentI al = new Alignment(new SequenceI[] { seq1, seq2 }); + + AlignFrame alignFrame = new AlignFrame(al, al.getWidth(), + al.getHeight()); + ScalePanel scalePanel = new ScalePanel( + alignFrame.getViewport(), alignFrame.alignPanel + ); + + MouseEvent mouse = new MouseEvent( + scalePanel, 0, 1, 0, 4, 0, 1, false + ); + scalePanel.mousePressed(mouse); + scalePanel.mouseDragged(mouse); + + // simulate dragging selection leftwards beyond the sequences giving + // negative X + mouse = new MouseEvent(scalePanel, 0, 1, 0, -30, 0, 1, false); + + scalePanel.mouseReleased(mouse); + + SequenceGroup sg = scalePanel.av.getSelectionGroup(); + int startCol = sg.getStartRes(); + + assertTrue(startCol >= 0); + + + } + +} diff --git a/test/jalview/io/IdentifyFileTest.java b/test/jalview/io/IdentifyFileTest.java index dd4f6ba..5be7968 100644 --- a/test/jalview/io/IdentifyFileTest.java +++ b/test/jalview/io/IdentifyFileTest.java @@ -94,7 +94,7 @@ public class IdentifyFileTest { "examples/plantfdx.fa", FileFormat.Fasta }, { "examples/dna_interleaved.phy", FileFormat.Phylip }, { "examples/2GIS.pdb", FileFormat.PDB }, - { "examples/rf00031_folded.stk", FileFormat.Stockholm }, + { "examples/RF00031_folded.stk", FileFormat.Stockholm }, { "examples/testdata/test.rnaml", FileFormat.Rnaml }, { "examples/testdata/test.aln", FileFormat.Clustal }, { "examples/testdata/test.pfam", FileFormat.Pfam }, diff --git a/test/jalview/io/cache/JvCacheableInputBoxTest.java b/test/jalview/io/cache/JvCacheableInputBoxTest.java index dfd7973..010a4b2 100644 --- a/test/jalview/io/cache/JvCacheableInputBoxTest.java +++ b/test/jalview/io/cache/JvCacheableInputBoxTest.java @@ -55,10 +55,9 @@ public class JvCacheableInputBoxTest cacheBox.updateCache(); try { - // This 1ms delay is essential to prevent the - // assertion below from executing before - // cacheBox.updateCache() finishes updating the cache - Thread.sleep(100); + // This delay is to let + // cacheBox.updateCache() finish updating the cache + Thread.sleep(200); } catch (InterruptedException e) { e.printStackTrace(); diff --git a/test/jalview/schemes/FeatureColourTest.java b/test/jalview/schemes/FeatureColourTest.java index 2eb718b..a96caec 100644 --- a/test/jalview/schemes/FeatureColourTest.java +++ b/test/jalview/schemes/FeatureColourTest.java @@ -523,11 +523,60 @@ public class FeatureColourTest assertFalse(fc.hasThreshold()); assertEquals(Color.RED, fc.getMinColour()); assertEquals(Color.GREEN, fc.getMaxColour()); + assertEquals(Color.RED, fc.getNoColour()); assertEquals(10f, fc.getMin()); assertEquals(20f, fc.getMax()); assertTrue(fc.isAutoScaled()); /* + * the same, with 'no value colour' specified as max + */ + fc = FeatureColour + .parseJalviewFeatureColour("red|green|novaluemax|10.0|20.0"); + assertEquals(Color.RED, fc.getMinColour()); + assertEquals(Color.GREEN, fc.getMaxColour()); + assertEquals(Color.GREEN, fc.getNoColour()); + assertEquals(10f, fc.getMin()); + assertEquals(20f, fc.getMax()); + + /* + * the same, with 'no value colour' specified as min + */ + fc = FeatureColour + .parseJalviewFeatureColour("red|green|novalueMin|10.0|20.0"); + assertEquals(Color.RED, fc.getMinColour()); + assertEquals(Color.GREEN, fc.getMaxColour()); + assertEquals(Color.RED, fc.getNoColour()); + assertEquals(10f, fc.getMin()); + assertEquals(20f, fc.getMax()); + + /* + * the same, with 'no value colour' specified as none + */ + fc = FeatureColour + .parseJalviewFeatureColour("red|green|novaluenone|10.0|20.0"); + assertEquals(Color.RED, fc.getMinColour()); + assertEquals(Color.GREEN, fc.getMaxColour()); + assertNull(fc.getNoColour()); + assertEquals(10f, fc.getMin()); + assertEquals(20f, fc.getMax()); + + /* + * the same, with invalid 'no value colour' + */ + try + { + fc = FeatureColour + .parseJalviewFeatureColour("red|green|blue|10.0|20.0"); + fail("expected exception"); + } catch (IllegalArgumentException e) + { + assertEquals( + "Couldn't parse the minimum value for graduated colour ('blue')", + e.getMessage()); + } + + /* * graduated colour (explicitly by 'score') (no threshold) */ fc = FeatureColour diff --git a/test/jalview/viewmodel/styles/ViewStyleTest.java b/test/jalview/viewmodel/styles/ViewStyleTest.java index 26c3574..885cb71 100644 --- a/test/jalview/viewmodel/styles/ViewStyleTest.java +++ b/test/jalview/viewmodel/styles/ViewStyleTest.java @@ -68,7 +68,7 @@ public class ViewStyleTest for (Field field : fields) { field.setAccessible(true); - if (!copyConstructorIgnores(field.getName())) + if (!copyConstructorIgnores(field)) { changeValue(vs1, field); } @@ -78,37 +78,49 @@ public class ViewStyleTest for (Field field1 : fields) { - final Object value1 = field1.get(vs1); - final Object value2 = field1.get(vs2); - String msg = "Mismatch in " + field1.getName() + "(" + value1 + "/" + if (!copyConstructorIgnores(field1)) + { + final Object value1 = field1.get(vs1); + final Object value2 = field1.get(vs2); + String msg = "Mismatch in " + field1.getName() + "(" + value1 + "/" + value2 + ") - not set in copy constructor?"; - assertEquals(msg, value1, value2); + assertEquals(msg, value1, value2); + } } assertEquals("Hashcode not equals", vs1.hashCode(), vs2.hashCode()); } /** - * Add any field names in here that we expect to be ignored by the copy - * constructor + * Add tests here for any fields that we expect to be ignored by + * the copy constructor * - * @param name + * @param field * @return */ - private boolean copyConstructorIgnores(String name) + private boolean copyConstructorIgnores(Field field) { /* - * currently none! + * ignore instrumentation added by jacoco for test coverage */ + if (field.isSynthetic()) + { + return true; + } + if (field.getType().toString().contains("com_atlassian_clover")) + { + return true; + } + return false; } - + /** - * Change the value of one field in a ViewStyle object - * - * @param vs - * @param field - * @throws IllegalAccessException - */ + * Change the value of one field in a ViewStyle object + * + * @param vs + * @param field + * @throws IllegalAccessException + */ protected void changeValue(ViewStyle vs, Field field) throws IllegalAccessException { @@ -210,19 +222,22 @@ public class ViewStyleTest Field[] fields = ViewStyle.class.getDeclaredFields(); for (Field field : fields) { - field.setAccessible(true); - Object oldValue = field.get(vs2); - changeValue(vs2, field); - assertFalse("equals method ignores " + field.getName(), + if (!copyConstructorIgnores(field)) + { + field.setAccessible(true); + Object oldValue = field.get(vs2); + changeValue(vs2, field); + assertFalse("equals method ignores " + field.getName(), vs1.equals(vs2)); - if (vs1.hashCode() == vs2.hashCode()) - { - // uncomment next line to see which fields hashCode ignores - // System.out.println("hashCode ignores " + field.getName()); + if (vs1.hashCode() == vs2.hashCode()) + { + // uncomment next line to see which fields hashCode ignores + // System.out.println("hashCode ignores " + field.getName()); + } + // restore original value before testing the next field + field.set(vs2, oldValue); } - // restore original value before testing the next field - field.set(vs2, oldValue); } } } diff --git a/utils/checkstyle/README.txt b/utils/checkstyle/README.txt index e38064e..54f4e48 100644 --- a/utils/checkstyle/README.txt +++ b/utils/checkstyle/README.txt @@ -1,6 +1,8 @@ Checkstyle for Jalview ---------------------- +See +https://issues.jalview.org/browse/JAL-1854 http://checkstyle.sourceforge.net/ GNU LGPL @@ -9,8 +11,16 @@ To get the Eclipse Checkstyle plugin - Help | Eclipse Marketplace - search for checkstyle - install eclipse-cs checkstyle plugin -The current version is 6.19.1 (August 2016). +Change Log +---------- +See http://checkstyle.sourceforge.net/releasenotes.html +Aug 2016 Initial version used is 6.19.1 +Dec 2018 Updated to 8.12.0 (latest on Eclipse Marketplace, 8.15 is latest release) + SuppressionCommentFilter relocated (changed in 8.1) + FileContentsHolder removed (changed in 8.2) + Updates to import-control.xml for code changes (htsjdk, stackoverflowusers) + Config ------ @@ -37,6 +47,7 @@ How to use checkstyle Option 2: on demand on selected code - right-click on a class or package and Checkstyle | Check code with checkstyle - (or Clear Checkstyle violations to remove checkstyle warnings) + - recommended to use this as a QA step when changing or reviewing code Checkstyle rules ---------------- @@ -64,11 +75,11 @@ Suppressing findings Tips ---- Sometimes checkstyle needs a kick before it will refresh its findings. - A whitespace edit in checkstyle.xml usually does this. There may be better ways. + Click the 'refresh' icon at top right in Eclipse | Preferences | Checkstyle. Invalid configuration files may result in checkstyle failing with an error reported in the Eclipse log file. - Help | Installation Details | Configuration takes you to a screen with a + Eclipse | About | Installation Details | Configuration takes you to a screen with a 'View Error Log' button. Sometimes checkstyle can fail silently. Try 'touching' (editing) config files, failing diff --git a/utils/checkstyle/checkstyle-suppress.xml b/utils/checkstyle/checkstyle-suppress.xml index 122b8d0..29a3047 100644 --- a/utils/checkstyle/checkstyle-suppress.xml +++ b/utils/checkstyle/checkstyle-suppress.xml @@ -26,6 +26,7 @@ + - - - - - - - @@ -54,10 +42,16 @@ - + + + + + diff --git a/utils/checkstyle/import-control.xml b/utils/checkstyle/import-control.xml index c47aaec..478966f 100644 --- a/utils/checkstyle/import-control.xml +++ b/utils/checkstyle/import-control.xml @@ -94,6 +94,7 @@ + @@ -116,6 +117,9 @@ + + +