From 5ed792c6889e7d77eb3008206518b2a3155ac00f Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Wed, 9 Nov 2016 16:58:33 +0000
Subject: [PATCH] JAL-2295 set attributes commands include feature value, sent
 via file

---
 src/jalview/ext/rbvi/chimera/ChimeraCommands.java  |   92 ++++++++++++++------
 .../ext/rbvi/chimera/JalviewChimeraBinding.java    |   10 +--
 2 files changed, 72 insertions(+), 30 deletions(-)

diff --git a/src/jalview/ext/rbvi/chimera/ChimeraCommands.java b/src/jalview/ext/rbvi/chimera/ChimeraCommands.java
index d34ac25..a5ee933 100644
--- a/src/jalview/ext/rbvi/chimera/ChimeraCommands.java
+++ b/src/jalview/ext/rbvi/chimera/ChimeraCommands.java
@@ -33,10 +33,13 @@ import jalview.util.Comparison;
 
 import java.awt.Color;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
+import MCview.PDBChain;
+
 /**
  * Routines for generating Chimera commands for Jalview/Chimera binding
  * 
@@ -297,7 +300,7 @@ public class ChimeraCommands
           StructureSelectionManager ssm, String[] files,
           SequenceI[][] seqs, FeatureRenderer fr, AlignmentI alignment)
   {
-    Map<String, AtomSpecModel> featureMap = buildFeaturesMap(
+    Map<String, Map<Object, AtomSpecModel>> featureMap = buildFeaturesMap(
             ssm, files, seqs, fr, alignment);
 
     List<String> commands = buildSetAttributeCommands(featureMap);
@@ -310,7 +313,10 @@ public class ChimeraCommands
   }
 
   /**
-   * Helper method to build a map of { featureType, AtomSpecModel }
+   * <pre>
+   * Helper method to build a map of 
+   *   { featureType, { feature value, AtomSpecModel } }
+   * </pre>
    * 
    * @param ssm
    * @param files
@@ -319,11 +325,11 @@ public class ChimeraCommands
    * @param alignment
    * @return
    */
-  protected static Map<String, AtomSpecModel> buildFeaturesMap(
+  protected static Map<String, Map<Object, AtomSpecModel>> buildFeaturesMap(
           StructureSelectionManager ssm, String[] files,
           SequenceI[][] seqs, FeatureRenderer fr, AlignmentI alignment)
   {
-    Map<String, AtomSpecModel> theMap = new LinkedHashMap<String, AtomSpecModel>();
+    Map<String, Map<Object, AtomSpecModel>> theMap = new LinkedHashMap<String, Map<Object, AtomSpecModel>>();
 
     List<String> visibleFeatures = fr.getDisplayedFeatureTypes();
     if (visibleFeatures.isEmpty())
@@ -376,7 +382,7 @@ public class ChimeraCommands
    */
   protected static void scanSequenceFeatures(List<String> visibleFeatures,
           StructureMapping mapping, SequenceI seq,
-          Map<String, AtomSpecModel> theMap, int modelNumber)
+          Map<String, Map<Object, AtomSpecModel>> theMap, int modelNumber)
   {
     SequenceFeature[] sfs = seq.getSequenceFeatures();
     if (sfs == null)
@@ -387,7 +393,7 @@ public class ChimeraCommands
     for (SequenceFeature sf : sfs)
     {
       String type = sf.getType();
-      if (!visibleFeatures.contains(type))
+      if (!visibleFeatures.contains(type) || suppressFeature(type))
       {
         continue;
       }
@@ -396,15 +402,25 @@ public class ChimeraCommands
 
       if (!mappedRanges.isEmpty())
       {
-        AtomSpecModel atomSpec = theMap.get(type);
-        if (atomSpec == null)
+        String value = sf.getDescription();
+        if (value == null)
+        {
+          value = type;
+        }
+        float score = sf.getScore();
+        if (score != 0f && score != Float.NaN)
         {
-          atomSpec = new AtomSpecModel();
-          theMap.put(type, atomSpec);
+          value = Float.toString(score);
+        }
+        Map<Object, AtomSpecModel> featureValues = theMap.get(type);
+        if (featureValues == null)
+        {
+          featureValues = new HashMap<Object, AtomSpecModel>();
+          theMap.put(type, featureValues);
         }
         for (int[] range : mappedRanges)
         {
-          atomSpec.addRange(modelNumber, range[0], range[1],
+          addRange(featureValues, value, modelNumber, range[0], range[1],
                   mapping.getChain());
         }
       }
@@ -412,34 +428,60 @@ public class ChimeraCommands
   }
 
   /**
-   * Traverse the map of features/models/chains/positions to construct a list of
-   * 'setattr' commands (one per feature type). The format of each command is
+   * Answers true if the feature type is one we don't wish to propagate to
+   * Chimera - for now, RESNUM
+   * 
+   * @param type
+   * @return
+   */
+  static boolean suppressFeature(String type)
+  {
+    return PDBChain.RESNUM_FEATURE.equals(type);
+  }
+
+  /**
+   * Traverse the map of features/values/models/chains/positions to construct a
+   * list of 'setattr' commands (one per distinct feature type and value).
+   * <p>
+   * The format of each command is
    * 
    * <pre>
    * <blockquote> setattr r <featureName> " " #modelnumber:range.chain 
-   * e.g. setattr r jv:chain " " #0:2.B,4.B,9-12.B|#1:1.A,2-6.A,...
+   * e.g. setattr r jv:chain <value> #0:2.B,4.B,9-12.B|#1:1.A,2-6.A,...
    * </blockquote>
    * </pre>
-   * <p>
-   * Note we are not (currently) setting attribute values, only the type
-   * (presence) of each attribute. This is to avoid overloading the Chimera REST
-   * interface by sending too many distinct commands. Analysis by feature values
-   * may still be performed in Jalview, on selections created in Chimera.
    * 
    * @param featureMap
    * @return
    */
   protected static List<String> buildSetAttributeCommands(
-          Map<String, AtomSpecModel> featureMap)
+          Map<String, Map<Object, AtomSpecModel>> featureMap)
   {
     List<String> commands = new ArrayList<String>();
     for (String featureType : featureMap.keySet())
     {
-      StringBuilder sb = new StringBuilder(128);
-      String sanitised = featureType.replace(" ", "_").replace("-", "_");
-      sb.append("setattr r jv_").append(sanitised).append(" \" \" ");
-      sb.append(featureMap.get(featureType).getAtomSpec());
-      commands.add(sb.toString());
+      String attributeName = "jv_"
+              + featureType.replace(" ", "_").replace("-", "_");
+
+      /*
+       * clear down existing attributes for this feature
+       */
+      // 'problem' - sets attribute to None on all residues - overkill?
+      // commands.add("~setattr r " + attributeName + " :*");
+
+      Map<Object, AtomSpecModel> values = featureMap.get(featureType);
+      for (Object value : values.keySet())
+      {
+        /*
+         * for each distinct value recorded for this feature type,
+         * add a command to set the attribute on the mapped residues
+         */
+        StringBuilder sb = new StringBuilder(128);
+        sb.append("setattr r ").append(attributeName).append(" \"")
+                .append(value.toString()).append("\" ");
+        sb.append(values.get(value).getAtomSpec());
+        commands.add(sb.toString());
+      }
     }
 
     return commands;
diff --git a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
index d94a3c2..8601b1b 100644
--- a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
+++ b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
@@ -1122,7 +1122,6 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
    */
   public void sendFeaturesToChimera(AlignmentViewPanel avp)
   {
-    // TODO send a command per feature with the range of residues it applies to
     AlignmentI alignment = avp.getAlignment();
     FeatureRenderer fr = getFeatureRenderer(avp);
 
@@ -1143,10 +1142,11 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
     StructureMappingcommandSet commandSet = ChimeraCommands
             .getSetAttributeCommandsForFeatures(getSsm(), files,
                     getSequence(), fr, alignment);
-    for (String command : commandSet.commands)
-    {
-      sendAsynchronousCommand(command, null);
-    }
+    // for (String command : commandSet.commands)
+    // {
+    // sendAsynchronousCommand(command, null);
+    // }
+    sendCommandsByFile(commandSet.commands);
   }
 
   /**
-- 
1.7.10.2