From 0a5c498eeb5d5b6bd73a3d1a9dc153f4de644fad Mon Sep 17 00:00:00 2001 From: Mateusz Warowny Date: Wed, 20 Oct 2021 12:54:13 +0200 Subject: [PATCH] JAL-3878 Annotation comments and questions --- .../ws2/operations/AnnotationOperation.java | 5 ++ .../ws2/operations/AnnotationServiceWorker.java | 52 ++++++++++++++------ 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/jalview/ws2/operations/AnnotationOperation.java b/src/jalview/ws2/operations/AnnotationOperation.java index b2e3fc6..31ac0a8 100644 --- a/src/jalview/ws2/operations/AnnotationOperation.java +++ b/src/jalview/ws2/operations/AnnotationOperation.java @@ -40,6 +40,10 @@ public class AnnotationOperation implements Operation boolean interactive = false; + /* + * Is it fine to get rid of AlignAnalysisUIText? + */ + public AnnotationOperation(WebServiceI service, ResultSupplier> annotSupplier, ResultSupplier featSupplier, String operationName) @@ -147,6 +151,7 @@ public class AnnotationOperation implements Operation var item = new JMenuItem(MessageManager.formatMessage( "label.calcname_with_default_settings", calcName)); item.addActionListener((event) -> { + /* What is the purpose of AlignViewport and AlignmentViewPanel? */ AlignViewport viewport = frame.getCurrentView(); AlignmentViewPanel alignPanel = frame.alignPanel; var worker = new AnnotationServiceWorker(this, service, diff --git a/src/jalview/ws2/operations/AnnotationServiceWorker.java b/src/jalview/ws2/operations/AnnotationServiceWorker.java index 10fc728..7f026a7 100644 --- a/src/jalview/ws2/operations/AnnotationServiceWorker.java +++ b/src/jalview/ws2/operations/AnnotationServiceWorker.java @@ -56,15 +56,18 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI private AlignFrame frame; private final AlignCalcManagerI2 calcMan; private Map seqNames; + /** + * indicates columns consisting of gaps only + */ boolean[] gapMap = new boolean[0]; int start, end; boolean transferSequenceFeatures = false; private WSJob job; private List ourAnnots; - + private int exceptionCount = MAX_RETRY; private static final int MAX_RETRY = 5; - + AnnotationServiceWorker(AnnotationOperation operation, WebServiceI service, List args, AlignViewport viewport, AlignmentViewPanel alignPanel, IProgressIndicator progressIndicator, AlignFrame frame, AlignCalcManagerI2 calcMan) @@ -78,7 +81,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI this.frame = frame; this.calcMan = calcMan; } - + @Override public String getCalcName() { @@ -96,6 +99,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI { if (!calcMan.isWorking(this) && job != null && !job.getStatus().isCompleted()) { + // is it correct to store annotations in a field and use them here? updateResultAnnotation(ourAnnots); } } @@ -129,6 +133,9 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI { return; } + /* What "bySequence" means in this context and + * what is the SelectionGroup and why is it only relevant when + * not dealing with alignment analysis? */ var bySequence = !operation.isAlignmentAnalysis(); sequences = prepareInput(viewport.getAlignment(), bySequence ? viewport.getSelectionGroup() : null); @@ -145,6 +152,8 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI service.getName())); job = new WSJob(service.getProviderName(), service.getName(), service.getHostName()); + // Should this part be moved out of this class to one of the gui + // classes? if (progressIndicator != null) { job.addPropertyChangeListener("status", new ProgressBarUpdater(progressIndicator)); @@ -173,7 +182,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI private List prepareInput(AlignmentI alignment, AnnotatedCollectionI inputSeqs) { - if (alignment == null || alignment.getWidth() <= 0 || + if (alignment == null || alignment.getWidth() <= 0 || alignment.getSequences() == null) return null; if (alignment.isNucleotide() && !operation.isNucleotideOperation()) @@ -183,12 +192,11 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI if (inputSeqs == null || inputSeqs.getWidth() <= 0 || inputSeqs.getSequences() == null || inputSeqs.getSequences().size() < 1) inputSeqs = alignment; - + List seqs = new ArrayList<>(); final boolean submitGaps = operation.isAlignmentAnalysis(); final int minlen = 10; - int ln = -1; - // FIXME don't return values by class parameters + int ln = -1; // I think this variable is redundant if (!operation.isAlignmentAnalysis()) seqNames = new HashMap<>(); start = inputSeqs.getStartRes(); @@ -200,7 +208,9 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI for (SequenceI sq : inputSeqs.getSequences()) { int sqlen; + // is it trying to find the length of a sequence excluding gaps? if (!operation.isAlignmentAnalysis()) + // why starting at positions to the right from the end/start? sqlen = sq.findPosition(end + 1) - sq.findPosition(start + 1); else sqlen = sq.getEnd() - sq.getStart(); @@ -221,7 +231,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI boolean[] tg = gapMap; gapMap = new boolean[seq.getLength()]; System.arraycopy(tg, 0, gapMap, 0, tg.length); - for (int p = tg.length; p < gapMap.length; p++) + for (int p = tg.length; p < gapMap.length; p++) { gapMap[p] = false; // init as a gap } @@ -241,7 +251,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI { // TODO: add ability to exclude hidden regions String sqstring = sq.getSequenceAsString(start, end + 1); - seq = new Sequence(newName, + seq = new Sequence(newName, AlignSeq.extractGaps(jalview.util.Comparison.GapChars, sqstring)); seqs.add(seq); // for annotation need to also record map to sequence start/end @@ -289,7 +299,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI } return seqs; } - + private boolean checkInputSequencesValid(List sequences) { int nvalid = 0; @@ -371,10 +381,13 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI FeaturesFile featuresFile; try { + // I think there should be a better way for obtaining features + // Are the features added to the sequences here? featuresFile = operation.featuresSupplier.getResult(job, sequences, viewport); if (featuresFile != null) { Alignment aln = new Alignment(sequences.toArray(new SequenceI[0])); + // I do nothing with the featureFilters object featuresFile.parse(aln, featureColours, true); } } catch (IOException e) @@ -391,6 +404,8 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI { aa.setCalcId(service.getName()); } + // Can't services other than alignment analysis be interactive? + // What's the point of storing that information in the annotation? aa.autoCalculated = operation.isAlignmentAnalysis() && operation.isInteractive(); } updateResultAnnotation(outputAnnotations); @@ -405,13 +420,13 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI { return featureColours.get(type); } - + @Override public FeatureMatcherSetI getFeatureFilters(String type) { return featureFilters.get(type); } - + @Override public boolean isFeatureDisplayed(String type) { @@ -427,13 +442,16 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI } Cache.log.debug("Annotation service task finished."); } - + + // What is the purpose of this method? + // When is it called (apart from the above)? private void updateResultAnnotation(List annotations) { var currentAnnotations = Objects.requireNonNullElse( viewport.getAlignment().getAlignmentAnnotation(), new AlignmentAnnotation[0]); List newAnnots = new ArrayList<>(); + // what is the graph group and why starting from 1? int graphGroup = 1; for (AlignmentAnnotation alna : currentAnnotations) { @@ -446,6 +464,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI ala.graphGroup += graphGroup; } + // stores original sequence, in what case it ends up as null? SequenceI aseq = null; if (ala.sequenceRef != null) { @@ -459,6 +478,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI Annotation[] resAnnot = ala.annotations; Annotation[] gappedAnnot = new Annotation[Math .max(viewport.getAlignment().getWidth(), gapMap.length)]; + // is it adding gaps which were previously removed to the annotation? for (int p = 0, ap = start; ap < gappedAnnot.length; ap++) { if (gapMap != null && gapMap.length > ap && !gapMap[ap]) @@ -470,6 +490,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI gappedAnnot[ap] = resAnnot[p++]; } } + // replacing sequence with the original one? ala.sequenceRef = aseq; ala.annotations = gappedAnnot; AlignmentAnnotation newAnnot = viewport.getAlignment() @@ -486,6 +507,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI for (SequenceI sq : sequences) { + // what are DBRefs? why are they relevant here? if (!sq.getFeatures().hasFeatures() && (sq.getDBRefs() == null || sq.getDBRefs().size() == 0)) { @@ -495,7 +517,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI SequenceI seq = seqNames.get(sq.getName()); SequenceI dseq; ContiguousI seqRange = seq.findPositions(start, end); - + while ((dseq = seq).getDatasetSequence() != null) { seq = seq.getDatasetSequence(); @@ -534,7 +556,7 @@ public class AnnotationServiceWorker implements PollableAlignCalcWorkerI } updateOurAnnots(newAnnots); } - + protected void updateOurAnnots(List annots) { List our = ourAnnots; -- 1.7.10.2