From: Ben Soares <b.soares@dundee.ac.uk>
Date: Fri, 3 Mar 2023 22:59:11 +0000 (+0000)
Subject: JAL-629 Improved consistency of ArgParser classes (use of ArgValuesMap). Added settin... 
X-Git-Tag: Release_2_11_4_0~460
X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=0fbfbe484308a0a6d9663f69f287cc61cfba958a;p=jalview.git

JAL-629 Improved consistency of ArgParser classes (use of ArgValuesMap). Added setting, checking and fetching ID value for ArgValues to allow easier linking of arguments.  Added argIndex for boolean Args.  Made noDuplicates (no duplicate values) an Arg Option.
---

diff --git a/src/jalview/bin/ArgParser.java b/src/jalview/bin/ArgParser.java
index be2c07f..cc31f5b 100644
--- a/src/jalview/bin/ArgParser.java
+++ b/src/jalview/bin/ArgParser.java
@@ -31,6 +31,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 
 import jalview.util.Platform;
 
@@ -42,7 +43,7 @@ public class ArgParser
 
   private static enum Opt
   {
-    BOOLEAN, STRING, UNARY, MULTI, LINKED, ORDERED
+    BOOLEAN, STRING, UNARY, MULTI, LINKED, ORDERED, NODUPLICATEVALUES
   }
 
   public enum Arg
@@ -207,6 +208,8 @@ public class ArgParser
 
   public static class ArgValues
   {
+    private static final String ID = "id";
+
     private Arg arg;
 
     private int argCount = 0;
@@ -215,16 +218,18 @@ public class ArgParser
 
     private boolean negated = false;
 
-    private int singleArgIndex = -1;
+    private int boolIndex = -1;
 
     private List<Integer> argsIndexes;
 
-    private List<ArgValue> argsList;
+    private List<ArgValue> argValueList;
+
+    private Map<String, ArgValue> idMap = new HashMap<>();
 
     protected ArgValues(Arg a)
     {
       this.arg = a;
-      this.argsList = new ArrayList<ArgValue>();
+      this.argValueList = new ArrayList<ArgValue>();
       this.boolValue = arg.getDefaultBoolValue();
     }
 
@@ -253,9 +258,10 @@ public class ArgParser
       return this.negated;
     }
 
-    protected void setBoolean(boolean b)
+    protected void setBoolean(boolean b, int i)
     {
       this.boolValue = b;
+      this.boolIndex = i;
     }
 
     protected boolean getBoolean()
@@ -266,7 +272,7 @@ public class ArgParser
     @Override
     public String toString()
     {
-      if (argsList == null)
+      if (argValueList == null)
         return null;
       StringBuilder sb = new StringBuilder();
       sb.append(arg.toLongString());
@@ -278,7 +284,7 @@ public class ArgParser
       {
         sb.append("Values:");
         boolean first = true;
-        for (ArgValue av : argsList)
+        for (ArgValue av : argValueList)
         {
           String v = av.getValue();
           if (!first)
@@ -300,44 +306,54 @@ public class ArgParser
 
     protected void addValue(String val, int argIndex)
     {
-      addValue(val, argIndex, false);
+      addArgValue(new ArgValue(val, argIndex));
     }
 
-    protected void addValue(String val, int argIndex, boolean noDuplicates)
+    protected void addArgValue(ArgValue av)
     {
-      if ((!arg.hasOption(Opt.MULTI) && argsList.size() > 0)
-              || (noDuplicates && argsList.contains(val)))
+      if ((!arg.hasOption(Opt.MULTI) && argValueList.size() > 0)
+              || (arg.hasOption(Opt.NODUPLICATEVALUES)
+                      && argValueList.contains(av.getValue())))
         return;
-      if (argsList == null)
+      if (argValueList == null)
+      {
+        argValueList = new ArrayList<ArgValue>();
+      }
+      SubVals sv = ArgParser.getSubVals(av.getValue());
+      if (sv.has(ID))
       {
-        argsList = new ArrayList<ArgValue>();
+        String id = sv.get(ID);
+        av.setId(id);
+        idMap.put(id, av);
       }
-      argsList.add(new ArgValue(val, argIndex));
+      argValueList.add(av);
     }
 
     protected boolean hasValue(String val)
     {
-      return argsList.contains(val);
+      return argValueList.contains(val);
     }
 
     protected ArgValue getArgValue()
     {
       if (arg.hasOption(Opt.MULTI))
         Console.warn("Requesting single value for multi value argument");
-      return argsList.size() > 0 ? argsList.get(0) : null;
+      return argValueList.size() > 0 ? argValueList.get(0) : null;
     }
 
-    /*
-    protected String getValue()
+    protected List<ArgValue> getArgValueList()
     {
-    ArgValue av = getArgValue();
-    return av == null ? null : av.getValue();
+      return argValueList;
     }
-    */
 
-    protected List<ArgValue> getArgValueList()
+    protected boolean hasId(String id)
+    {
+      return idMap.containsKey(id);
+    }
+
+    protected ArgValue getId(String id)
     {
-      return argsList;
+      return idMap.get(id);
     }
   }
 
@@ -371,7 +387,8 @@ public class ArgParser
   public String getValue(String arg, boolean utf8decode)
   {
     int index = vargs.indexOf(arg);
-    String dc = null, ret = null;
+    String dc = null;
+    String ret = null;
     if (index != -1)
     {
       ret = vargs.get(index + 1).toString();
@@ -405,7 +422,7 @@ public class ArgParser
   // new style
   private static final Map<String, Arg> argMap;
 
-  private Map<String, HashMap<Arg, ArgValues>> linkedArgs = new HashMap<>();
+  private Map<String, ArgValuesMap> linkedArgs = new HashMap<>();
 
   private List<String> linkedOrder = null;
 
@@ -556,32 +573,48 @@ public class ArgParser
           linkedId = DEFAULTLINKEDID;
 
         if (!linkedArgs.containsKey(linkedId))
-          linkedArgs.put(linkedId, new HashMap<>());
+          linkedArgs.put(linkedId, new ArgValuesMap());
 
-        Map<Arg, ArgValues> valuesMap = linkedArgs.get(linkedId);
-        if (!valuesMap.containsKey(a))
-          valuesMap.put(a, new ArgValues(a));
+        ArgValuesMap avm = linkedArgs.get(linkedId);
 
-        ArgValues values = valuesMap.get(a);
-        if (values == null)
+        if (a.hasOption(Opt.NODUPLICATEVALUES) && avm.hasValue(a, val))
         {
-          values = new ArgValues(a);
+          Console.error("Argument '--" + argName
+                  + "' cannot contain a duplicate value ('" + val
+                  + "'). Ignoring this and subsequent occurrences.");
+          continue;
+        }
+
+        // check for unique id
+        SubVals sv = ArgParser.getSubVals(val);
+        String id = sv.get(ArgValues.ID);
+        if (id != null && avm.hasId(a, id))
+        {
+          Console.error("Argument '--" + argName + "' has a duplicate id ('"
+                  + id + "'). Ignoring.");
+          continue;
+        }
+
+        ArgValues avs = avm.getOrCreateArgValues(a);
+        if (avs == null)
+        {
+          avs = new ArgValues(a);
         }
         // store appropriate value
         if (a.hasOption(Opt.STRING))
         {
-          values.addValue(val, argIndex);
+          avs.addValue(val, argIndex);
         }
         else if (a.hasOption(Opt.BOOLEAN))
         {
-          values.setBoolean(!negated);
-          values.setNegated(negated);
+          avs.setBoolean(!negated, argIndex);
+          avs.setNegated(negated);
         }
         else if (a.hasOption(Opt.UNARY))
         {
-          values.setBoolean(true);
+          avs.setBoolean(true, argIndex);
         }
-        values.incrementCount();
+        avs.incrementCount();
 
         // store in appropriate place
         if (a.hasOption(Opt.LINKED))
@@ -595,10 +628,8 @@ public class ArgParser
           if (!linkedOrder.contains(linkedId))
             linkedOrder.add(linkedId);
         }
-        // store the ArgValues
-        valuesMap.put(a, values);
 
-        // store arg in the list of args
+        // store arg in the list of args used
         if (argList == null)
           argList = new ArrayList<>();
         if (!argList.contains(a))
@@ -614,8 +645,8 @@ public class ArgParser
 
   public boolean isSet(String linkedId, Arg a)
   {
-    Map<Arg, ArgValues> m = linkedArgs.get(linkedId);
-    return m == null ? false : m.containsKey(a);
+    ArgValuesMap avm = linkedArgs.get(linkedId);
+    return avm == null ? false : avm.containsArg(a);
   }
 
   public boolean getBool(Arg a)
@@ -630,11 +661,11 @@ public class ArgParser
 
   public boolean getBool(String linkedId, Arg a)
   {
-    Map<Arg, ArgValues> m = linkedArgs.get(linkedId);
-    if (m == null)
+    ArgValuesMap avm = linkedArgs.get(linkedId);
+    if (avm == null)
       return a.getDefaultBoolValue();
-    ArgValues v = m.get(a);
-    return v == null ? a.getDefaultBoolValue() : v.getBoolean();
+    ArgValues avs = avm.getArgValues(a);
+    return avs == null ? a.getDefaultBoolValue() : avs.getBoolean();
   }
 
   public List<String> linkedIds()
@@ -642,7 +673,7 @@ public class ArgParser
     return linkedOrder;
   }
 
-  public HashMap<Arg, ArgValues> linkedArgs(String id)
+  public ArgValuesMap linkedArgs(String id)
   {
     return linkedArgs.get(id);
   }
@@ -652,7 +683,7 @@ public class ArgParser
   {
     StringBuilder sb = new StringBuilder();
     sb.append("UNLINKED\n");
-    sb.append(argMapToString(linkedArgs.get(null)));
+    sb.append(argValuesMapToString(linkedArgs.get(null)));
     if (linkedIds() != null)
     {
       sb.append("LINKED\n");
@@ -662,22 +693,22 @@ public class ArgParser
         if (id == null)
           continue;
 
-        Map<Arg, ArgValues> m = linkedArgs(id);
+        ArgValuesMap avm = linkedArgs(id);
         sb.append("ID: '").append(id).append("'\n");
-        sb.append(argMapToString(m));
+        sb.append(argValuesMapToString(avm));
       }
     }
     return sb.toString();
   }
 
-  private static String argMapToString(Map<Arg, ArgValues> m)
+  private static String argValuesMapToString(ArgValuesMap avm)
   {
-    if (m == null)
+    if (avm == null)
       return null;
     StringBuilder sb = new StringBuilder();
-    for (Arg a : m.keySet())
+    for (Arg a : avm.getArgKeys())
     {
-      ArgValues v = m.get(a);
+      ArgValues v = avm.getArgValues(a);
       sb.append(v.toString());
       sb.append("\n");
     }
@@ -698,6 +729,8 @@ public class ArgParser
 
     private String value;
 
+    private String id;
+
     protected ArgValue(String value, int argIndex)
     {
       this.value = value;
@@ -713,6 +746,16 @@ public class ArgParser
     {
       return argIndex;
     }
+
+    protected void setId(String i)
+    {
+      id = i;
+    }
+
+    protected String getId()
+    {
+      return id;
+    }
   }
 
   /**
@@ -817,20 +860,62 @@ public class ArgParser
   {
     protected Map<Arg, ArgValues> m;
 
+    protected ArgValuesMap()
+    {
+      this.newMap();
+    }
+
     protected ArgValuesMap(Map<Arg, ArgValues> map)
     {
       this.m = map;
     }
 
+    private Map<Arg, ArgValues> getMap()
+    {
+      return m;
+    }
+
+    private void newMap()
+    {
+      m = new HashMap<Arg, ArgValues>();
+    }
+
+    private void newArg(Arg a)
+    {
+      if (m == null)
+        newMap();
+      if (!containsArg(a))
+        m.put(a, new ArgValues(a));
+    }
+
+    protected void addArgValue(Arg a, ArgValue av)
+    {
+      if (getMap() == null)
+        m = new HashMap<Arg, ArgValues>();
+
+      if (!m.containsKey(a))
+        m.put(a, new ArgValues(a));
+      ArgValues avs = m.get(a);
+      avs.addArgValue(av);
+    }
+
     protected ArgValues getArgValues(Arg a)
     {
       return m == null ? null : m.get(a);
     }
 
+    protected ArgValues getOrCreateArgValues(Arg a)
+    {
+      ArgValues avs = m.get(a);
+      if (avs == null)
+        newArg(a);
+      return getArgValues(a);
+    }
+
     protected List<ArgValue> getArgValueList(Arg a)
     {
-      ArgValues av = getArgValues(a);
-      return av == null ? null : av.getArgValueList();
+      ArgValues avs = getArgValues(a);
+      return avs == null ? new ArrayList<>() : avs.getArgValueList();
     }
 
     protected ArgValue getArgValue(Arg a)
@@ -845,19 +930,40 @@ public class ArgParser
       return av == null ? null : av.getValue();
     }
 
-    protected boolean hasValue(Arg a)
+    protected boolean containsArg(Arg a)
     {
-      if (!m.containsKey(a))
+      if (m == null || !m.containsKey(a))
         return false;
       return getArgValue(a) != null;
     }
 
+    protected boolean hasValue(Arg a, String val)
+    {
+      if (m == null || !m.containsKey(a))
+        return false;
+      for (ArgValue av : getArgValueList(a))
+      {
+        String avVal = av.getValue();
+        if ((val == null && avVal == null)
+                || (val != null && val.equals(avVal)))
+        {
+          return true;
+        }
+      }
+      return false;
+    }
+
     protected boolean getBoolean(Arg a)
     {
       ArgValues av = getArgValues(a);
       return av == null ? false : av.getBoolean();
     }
 
+    protected Set<Arg> getArgKeys()
+    {
+      return m.keySet();
+    }
+
     protected ArgValue getClosestPreviousArgValueOfArg(ArgValue thisAv,
             Arg a)
     {
@@ -876,6 +982,59 @@ public class ArgParser
       }
       return closestAv;
     }
+
+    protected ArgValue getClosestNextArgValueOfArg(ArgValue thisAv, Arg a)
+    {
+      // this looks for the *next* arg that *might* be referring back to
+      // a thisAv. Such an arg would have no subValues (if it does it should
+      // specify an id in the subValues so wouldn't need to be guessed).
+      ArgValue closestAv = null;
+      int thisArgIndex = thisAv.getArgIndex();
+      ArgValues compareAvs = this.getArgValues(a);
+      int closestNextIndex = Integer.MAX_VALUE;
+      for (ArgValue av : compareAvs.getArgValueList())
+      {
+        int argIndex = av.getArgIndex();
+        if (argIndex > thisArgIndex && argIndex < closestNextIndex)
+        {
+          closestNextIndex = argIndex;
+          closestAv = av;
+        }
+      }
+      return closestAv;
+    }
+
+    protected ArgValue[] getArgValuesReferringTo(String key, String value,
+            Arg a)
+    {
+      // this looks for the *next* arg that *might* be referring back to
+      // a thisAv. Such an arg would have no subValues (if it does it should
+      // specify an id in the subValues so wouldn't need to be guessed).
+      List<ArgValue> avList = new ArrayList<>();
+      Arg[] args = a == null ? (Arg[]) this.getMap().keySet().toArray()
+              : new Arg[]
+              { a };
+      for (Arg keyArg : args)
+      {
+        for (ArgValue av : this.getArgValueList(keyArg))
+        {
+
+        }
+      }
+      return (ArgValue[]) avList.toArray();
+    }
+
+    protected boolean hasId(Arg a, String id)
+    {
+      ArgValues avs = this.getArgValues(a);
+      return avs == null ? false : avs.hasId(id);
+    }
+
+    protected ArgValue getId(Arg a, String id)
+    {
+      ArgValues avs = this.getArgValues(a);
+      return avs == null ? null : avs.getId(id);
+    }
   }
 
   private static final Collection<Arg> bootstrapArgs = new ArrayList(
diff --git a/src/jalview/bin/Commands.java b/src/jalview/bin/Commands.java
index a98069a..229df27 100644
--- a/src/jalview/bin/Commands.java
+++ b/src/jalview/bin/Commands.java
@@ -120,7 +120,7 @@ public class Commands
 
   protected void processLinked(String id)
   {
-    ArgValuesMap avm = new ArgValuesMap(argParser.linkedArgs(id));
+    ArgValuesMap avm = argParser.linkedArgs(id);
     if (avm == null)
       return;
     else
@@ -136,7 +136,7 @@ public class Commands
     FileFormatI format = null;
     DataSourceType protocol = null;
     */
-    if (avm.hasValue(Arg.OPEN))
+    if (avm.containsArg(Arg.OPEN))
     {
       long progress = -1;
 
@@ -224,7 +224,8 @@ public class Commands
 
           // get kind of temperature factor annotation
           StructureImportSettings.TFType tempfacType = TFType.DEFAULT;
-          if ((!avm.getBoolean(Arg.NOTEMPFAC)) && avm.hasValue(Arg.TEMPFAC))
+          if ((!avm.getBoolean(Arg.NOTEMPFAC))
+                  && avm.containsArg(Arg.TEMPFAC))
           {
             try
             {
@@ -269,13 +270,13 @@ public class Commands
           }
 
           // colour aligment?
-          if (avm.hasValue(Arg.COLOUR))
+          if (avm.containsArg(Arg.COLOUR))
           {
             af.changeColour_actionPerformed(avm.getValue(Arg.COLOUR));
           }
 
           // change alignment frame title
-          if (avm.hasValue(Arg.TITLE))
+          if (avm.containsArg(Arg.TITLE))
             af.setTitle(avm.getValue(Arg.TITLE));
 
           /* hacky approach to hiding the annotations */
@@ -305,7 +306,7 @@ public class Commands
            if (showTemperatureFactor)
              */
           {
-            if (avm.hasValue(Arg.TEMPFAC_LABEL))
+            if (avm.containsArg(Arg.TEMPFAC_LABEL))
             {
               AlignmentAnnotation aa = AlignmentUtils
                       .getFirstSequenceAnnotationOfType(
@@ -369,7 +370,7 @@ public class Commands
     if (!avm.getBoolean(Arg.NOSTRUCTURE))
     {
       AlignFrame af = afMap.get(id);
-      if (avm.hasValue(Arg.STRUCTURE))
+      if (avm.containsArg(Arg.STRUCTURE))
       {
         for (ArgValue av : avm.getArgValueList(Arg.STRUCTURE))
         {
@@ -443,14 +444,21 @@ public class Commands
             Cache.setProperty(Preferences.STRUCTURE_DISPLAY,
                     StructureViewer.ViewerType.JMOL.toString());
           }
-          StructureChooser.openStructureFileForSequence(ap, seq,
-                  structureFile);
+
+          // get tft, paeFilename, label?
+          /*
+          ArgValue tftAv = avm.getArgValuesReferringTo("structid", structId,
+                  Arg.TEMPFAC);
+           */
+          StructureChooser.openStructureFileForSequence(null, null, ap, seq,
+                  false, structureFile.getAbsolutePath(), null, null); // tft,
+                                                                       // paeFilename);
         }
       }
     }
 
     // load a pAE file if given
-    if (avm.hasValue(Arg.PAEMATRIX))
+    if (avm.containsArg(Arg.PAEMATRIX))
     {
       AlignFrame af = afMap.get(id);
       if (af != null)
@@ -526,7 +534,7 @@ public class Commands
 
   protected void processImages(String id)
   {
-    ArgValuesMap avm = new ArgValuesMap(argParser.linkedArgs(id));
+    ArgValuesMap avm = argParser.linkedArgs(id);
     AlignFrame af = afMap.get(id);
 
     if (af == null)
@@ -535,7 +543,7 @@ public class Commands
       return;
     }
 
-    if (avm.hasValue(Arg.IMAGE))
+    if (avm.containsArg(Arg.IMAGE))
     {
       for (ArgValue av : avm.getArgValueList(Arg.IMAGE))
       {
diff --git a/src/jalview/gui/StructureChooser.java b/src/jalview/gui/StructureChooser.java
index 80024d9..29551b8 100644
--- a/src/jalview/gui/StructureChooser.java
+++ b/src/jalview/gui/StructureChooser.java
@@ -1266,9 +1266,9 @@ public class StructureChooser extends GStructureChooser
         }
         else if (currentView == VIEWS_FROM_FILE)
         {
-          TFType tft = (TFType) StructureChooser.this.combo_tempFacAs
-                  .getSelectedItem();
-          String paeFilename = StructureChooser.this.localPdbPaeMatrixFileName;
+          StructureChooser sc = StructureChooser.this;
+          TFType tft = (TFType) sc.combo_tempFacAs.getSelectedItem();
+          String paeFilename = sc.localPdbPaeMatrixFileName;
           AssociateSeqOptions assSeqOpt = (AssociateSeqOptions) fileChooserAssSeqPanel
                   .getCmb_assSeq().getSelectedItem();
           SequenceI userSelectedSeq = assSeqOpt.getSequence();
@@ -1276,37 +1276,8 @@ public class StructureChooser extends GStructureChooser
             selectedSequence = userSelectedSeq;
           String pdbFilename = selectedPdbFileName;
 
-          PDBEntry fileEntry = new AssociatePdbFileWithSeq()
-                  .associatePdbWithSeq(pdbFilename, DataSourceType.FILE,
-                          selectedSequence, true, Desktop.instance, tft,
-                          paeFilename);
-
-          /*
-          SequenceI[] seqArray = new SequenceI[] { selectedSequence };
-          
-          StructureFile sf = ssm.computeMapping(true, seqArray, null,
-                  selectedPdbFileName, DataSourceType.FILE, null, tft,
-                  paeFilename);
-          StructureMapping[] sm = ssm.getMapping(fileEntry.getFile());
-          // DO SOMETHING WITH
-          File paeFile = paeFilename == null ? null : new File(paeFilename);
-          if (paeFilename != null && paeFile.exists())
-          {
-            AlignmentI al = StructureChooser.this.ap.getAlignment();
-            try
-            {
-              EBIAlfaFold.importPaeJSONAsContactMatrixToSequence(al,
-                      paeFile, selectedSequence);
-            } catch (IOException | ParseException e)
-            {
-              // TODO Auto-generated catch block
-              e.printStackTrace();
-            }
-          }
-          */
-          sViewer = launchStructureViewer(ssm, new PDBEntry[] { fileEntry },
-                  ap, new SequenceI[]
-                  { selectedSequence });
+          StructureChooser.openStructureFileForSequence(ssm, sc, null,
+                  selectedSequence, true, pdbFilename, tft, paeFilename);
         }
         SwingUtilities.invokeLater(new Runnable()
         {
@@ -1735,30 +1706,29 @@ public class StructureChooser extends GStructureChooser
   /**
    * Open a single structure file for a given sequence
    */
-  public static void openStructureFileForSequence(AlignmentPanel ap,
-          SequenceI seq, File sFile)
+  public static void openStructureFileForSequence(
+          StructureSelectionManager ssm, StructureChooser sc,
+          AlignmentPanel ap, SequenceI seq, boolean prompt,
+          String sFilename, TFType tft, String paeFilename)
   {
-    // Open the chooser headlessly. Not sure this is actually needed ?
-    StructureChooser sc = new StructureChooser(new SequenceI[] { seq }, seq,
-            ap, false);
-    StructureSelectionManager ssm = ap.getStructureSelectionManager();
-    PDBEntry fileEntry = null;
-    try
-    {
-      fileEntry = new AssociatePdbFileWithSeq().associatePdbWithSeq(
-              sFile.getAbsolutePath(), DataSourceType.FILE, seq, true,
-              Desktop.instance);
-    } catch (Exception e)
-    {
-      Console.error("Could not open structure file '"
-              + sFile.getAbsolutePath() + "'");
-      return;
+    boolean headless = false;
+    if (sc == null)
+    {
+      headless = true;
+      sc = new StructureChooser(new SequenceI[] { seq }, seq, ap, false);
     }
+    if (ssm == null)
+      ssm = ap.getStructureSelectionManager();
+
+    PDBEntry fileEntry = new AssociatePdbFileWithSeq().associatePdbWithSeq(
+            sFilename, DataSourceType.FILE, seq, prompt, Desktop.instance,
+            tft, paeFilename);
 
     StructureViewer sViewer = sc.launchStructureViewer(ssm,
             new PDBEntry[]
             { fileEntry }, ap, new SequenceI[] { seq });
 
-    sc.mainFrame.dispose();
+    if (headless)
+      sc.mainFrame.dispose();
   }
 }