JAL-3878 Annotation comments and questions

diff --git a/src/jalview/ws2/operations/AnnotationOperation.java b/src/jalview/ws2/operations/AnnotationOperation.java
index b2e3fc6..31ac0a8 100644 (file)
--- 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<List<AlignmentAnnotation>> annotSupplier,
       ResultSupplier<FeaturesFile> 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 (file)
--- 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<String, SequenceI> seqNames;
+  /**
+   * indicates columns consisting of gaps only
+   */
   boolean[] gapMap = new boolean[0];
   int start, end;
   boolean transferSequenceFeatures = false;
   private WSJob job;
   private List<AlignmentAnnotation> ourAnnots;
-  
+
   private int exceptionCount = MAX_RETRY;
   private static final int MAX_RETRY = 5;
-  
+
   AnnotationServiceWorker(AnnotationOperation operation, WebServiceI service,
       List<ArgumentI> 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<SequenceI> 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<SequenceI> 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<SequenceI> 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<AlignmentAnnotation> annotations)
   {
     var currentAnnotations = Objects.requireNonNullElse(
         viewport.getAlignment().getAlignmentAnnotation(),
         new AlignmentAnnotation[0]);
     List<AlignmentAnnotation> 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<AlignmentAnnotation> annots)
   {
     List<AlignmentAnnotation> our = ourAnnots;