From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Wed, 16 Nov 2016 12:23:33 +0000 (+0000)
Subject: Merge branch 'develop' into features/JAL-2295setChimeraAttributes
X-Git-Tag: Release_2_11_0~62^2~45
X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=58951652adedb7c15569b5295359baa4c3a81e1e;p=jalview.git

Merge branch 'develop' into features/JAL-2295setChimeraAttributes

Conflicts:
	src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
	src/jalview/gui/ChimeraViewFrame.java
---

58951652adedb7c15569b5295359baa4c3a81e1e
diff --cc src/jalview/datamodel/SequenceFeature.java
index c75d6f2,c75d6f2..0baa78e
--- a/src/jalview/datamodel/SequenceFeature.java
+++ b/src/jalview/datamodel/SequenceFeature.java
@@@ -208,7 -208,7 +208,9 @@@ public class SequenceFeatur
      }
  
      SequenceFeature sf = (SequenceFeature) o;
--    if (begin != sf.begin || end != sf.end || score != sf.score)
++    boolean sameScore = Float.isNaN(score) ? Float.isNaN(sf.score)
++            : score == sf.score;
++    if (begin != sf.begin || end != sf.end || !sameScore)
      {
        return false;
      }
diff --cc src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
index 18a614d,4a9bf5f..72b33cc
--- a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
+++ b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
@@@ -41,13 -38,9 +41,14 @@@ import jalview.structures.models.AAStru
  import jalview.util.MessageManager;
  
  import java.awt.Color;
 +import java.io.File;
 +import java.io.FileOutputStream;
 +import java.io.IOException;
 +import java.io.PrintWriter;
  import java.net.BindException;
  import java.util.ArrayList;
 +import java.util.Collections;
+ import java.util.Hashtable;
  import java.util.LinkedHashMap;
  import java.util.List;
  import java.util.Map;
@@@ -59,10 -52,6 +60,8 @@@ import ext.edu.ucsf.rbvi.strucviz2.Stru
  
  public abstract class JalviewChimeraBinding extends AAStructureBindingModel
  {
 +  public static final String CHIMERA_FEATURE_GROUP = "Chimera";
 +
-   private static final String CHIMERA_FEATURE_PREFIX = "chim_";
- 
    // Chimera clause to exclude alternate locations in atom selection
    private static final String NO_ALTLOCS = "&~@.B-Z&~@.2-9";
  
@@@ -75,6 -64,9 +74,10 @@@
  
    private static final String ALPHACARBON = "CA";
  
+   private List<String> chainNames = new ArrayList<String>();
+ 
+   private Hashtable<String, String> chainFile = new Hashtable<String, String>();
++  
    /*
     * Object through which we talk to Chimera
     */
@@@ -191,7 -191,7 +194,6 @@@
     * @param ssm
     * @param pdbentry
     * @param sequenceIs
--   * @param chains
     * @param protocol
     */
    public JalviewChimeraBinding(StructureSelectionManager ssm,
@@@ -723,30 -731,30 +727,6 @@@
     */
    public abstract void refreshPdbEntries();
  
--  private int getModelNum(String modelFileName)
--  {
--    String[] mfn = getPdbFile();
--    if (mfn == null)
--    {
--      return -1;
--    }
--    for (int i = 0; i < mfn.length; i++)
--    {
--      if (mfn[i].equalsIgnoreCase(modelFileName))
--      {
--        return i;
--      }
--    }
--    return -1;
--  }
--
--  /**
--   * map between index of model filename returned from getPdbFile and the first
--   * index of models from this file in the viewer. Note - this is not trimmed -
--   * use getPdbFile to get number of unique models.
--   */
--  private int _modelFileNameMap[];
--
    // ////////////////////////////////
    // /StructureListener
    @Override
@@@ -1070,28 -1061,6 +1038,11 @@@
     * 
     * @return
     */
++  @Override
 +  public List<String> getChainNames()
 +  {
-     List<String> names = new ArrayList<String>();
-     String[][] allNames = getChains();
-     if (allNames != null)
-     {
-       for (String[] chainsForPdb : allNames)
-       {
-         if (chainsForPdb != null)
-         {
-           for (String chain : chainsForPdb)
-           {
-             if (chain != null && !names.contains(chain))
-             {
-               names.add(chain);
-             }
-           }
-         }
-       }
-     }
-     return names;
++    return chainNames;
 +  }
  
    /**
     * Send a 'focus' command to Chimera to recentre the visible display
@@@ -1129,226 -1098,30 +1080,217 @@@
      }
    }
  
 +  /**
 +   * Constructs and send commands to Chimera to set attributes on residues for
 +   * features visible in Jalview
 +   * 
 +   * @param avp
 +   */
 +  public void sendFeaturesToViewer(AlignmentViewPanel avp)
 +  {
 +    // TODO refactor as required to pull up to an interface
 +    AlignmentI alignment = avp.getAlignment();
 +    FeatureRenderer fr = getFeatureRenderer(avp);
  
 -  @Override
 -  public List<String> getChainNames()
 +    /*
 +     * fr is null if feature display is turned off
 +     */
 +    if (fr == null)
 +    {
 +      return;
 +    }
 +
 +    String[] files = getPdbFile();
 +    if (files == null)
 +    {
 +      return;
 +    }
 +
 +    StructureMappingcommandSet commandSet = ChimeraCommands
 +            .getSetAttributeCommandsForFeatures(getSsm(), files,
 +                    getSequence(), fr, alignment);
 +    String[] commands = commandSet.commands;
 +    if (commands.length > 10)
 +    {
 +      sendCommandsByFile(commands);
 +    }
 +    else
 +    {
 +      for (String command : commands)
 +      {
 +        sendAsynchronousCommand(command, null);
 +      }
 +    }
 +  }
 +
 +  /**
 +   * Write commands to a temporary file, and send a command to Chimera to open
 +   * the file as a commands script. For use when sending a large number of
 +   * separate commands would overload the REST interface mechanism.
 +   * 
 +   * @param commands
 +   */
 +  protected void sendCommandsByFile(String[] commands)
    {
 -    return chainNames;
 +    try
 +    {
 +      File tmp = File.createTempFile("chim", ".com");
 +      tmp.deleteOnExit();
 +      PrintWriter out = new PrintWriter(new FileOutputStream(tmp));
 +      for (String command : commands)
 +      {
 +        out.println(command);
 +      }
 +      out.flush();
 +      out.close();
 +      String path = tmp.getAbsolutePath();
 +      sendAsynchronousCommand("open cmd:" + path, null);
 +    } catch (IOException e)
 +    {
 +      System.err
 +              .println("Sending commands to Chimera via file failed with "
 +                      + e.getMessage());
 +    }
    }
  
 +  /**
 +   * Get Chimera residues which have the named attribute, find the mapped
 +   * positions in the Jalview sequence(s), and set as sequence features
 +   * 
 +   * @param attName
 +   * @param alignmentPanel
 +   */
 +  public void copyStructureAttributesToFeatures(String attName,
 +          AlignmentViewPanel alignmentPanel)
 +  {
 +    // todo pull up to AAStructureBindingModel (and interface?)
 +
 +    /*
 +     * ask Chimera to list residues with the attribute, reporting its value
 +     */
 +    // this alternative command
 +    // list residues spec ':*/attName' attr attName
 +    // doesn't report 'None' values (which is good), but
 +    // fails for 'average.bfactor' (which is bad):
 +
 +    String cmd = "list residues attr '" + attName + "'";
 +    List<String> residues = sendChimeraCommand(cmd, true);
- 
-     /*
-      * TODO check if Jalview already has this feature name, if so give it a 
-      * distinguishing prefix e.g. chim_
-      */
-     FeatureRenderer fr = alignmentPanel.getFeatureRenderer();
-     // todo should getRenderOrder be in api.FeatureRenderer?
-     // FIXME this is empty if feature display is turned off
-     List<String> existingFeatures = ((jalview.gui.FeatureRenderer) fr)
-             .getRenderOrder();
-     if (existingFeatures.contains(attName))
-     {
-       // TODO check if feature of this name is in group Chimera
-       // if so don't create a new feature name!
-       // problem: needs a lookup of features for feature group
-       attName = getStructureFeaturePrefix() + attName;
-     }
- 
 +    /*
 +     * Expect 0, 1 or more reply lines of the format (chi2 is attName):
 +     * residue id #0:5.A chi2 -155.000836316 index 5
 +     * or
 +     * residue id #0:6.A chi3 None
 +     * 
 +     * We assume here that attributes on structure do not naturally convert
 +     * to ranges on sequence, i.e. we just set one feature per mapped position.
 +     * 
 +     * To conflate positions, would need to first build a map 
 +     * Map<String value, Map<Sequence seq, List<Integer position>>>
 +     * and then traverse it to find feature ranges.
 +     */
 +    boolean featureAdded = false;
 +    String featureGroup = getViewerFeatureGroup();
 +
 +    for (String residue : residues)
 +    {
 +      AtomSpec spec = null;
 +      String[] tokens = residue.split(" ");
 +      if (tokens.length < 5)
 +      {
 +        continue;
 +      }
 +      String atomSpec = tokens[2];
 +      String attValue = tokens[4];
 +
 +      /*
 +       * ignore 'None' (e.g. for phi) or 'False' (e.g. for isHelix)
 +       */
 +      if ("None".equalsIgnoreCase(attValue)
 +              || "False".equalsIgnoreCase(attValue))
 +      {
 +        continue;
 +      }
 +
 +      try
 +      {
 +        spec = AtomSpec.fromChimeraAtomspec(atomSpec);
 +      } catch (IllegalArgumentException e)
 +      {
 +        System.err.println("Problem parsing atomspec " + atomSpec);
 +        continue;
 +      }
 +
 +      String chainId = spec.getChain();
 +      String description = attValue;
 +      float score = Float.NaN;
 +      try
 +      {
 +        score = Float.valueOf(attValue);
 +        description = chainId;
 +      } catch (NumberFormatException e)
 +      {
 +        // was not a float value
 +      }
 +
 +      String pdbFile = getPdbFileForModel(spec.getModelNumber());
 +      spec.setPdbFile(pdbFile);
 +
 +      List<AtomSpec> atoms = Collections.singletonList(spec);
 +      SearchResults sr = getSsm()
 +              .findAlignmentPositionsForStructurePositions(atoms);
 +
 +      /*
 +       * expect one matched alignment position, or none 
 +       * (if the structure position is not mapped)
 +       */
 +      for (Match m : sr.getResults())
 +      {
 +        SequenceI seq = m.getSequence();
 +        int start = m.getStart();
 +        int end = m.getEnd();
 +        SequenceFeature sf = new SequenceFeature(attName, description,
 +                start, end, score, featureGroup);
 +        // todo: should SequenceFeature have an explicit property for chain?
 +        // note: repeating the action shouldn't duplicate features
 +        featureAdded |= seq.addSequenceFeature(sf);
 +      }
 +    }
 +    if (featureAdded)
 +    {
-       fr.featuresAdded();
++      alignmentPanel.getFeatureRenderer().featuresAdded();
 +    }
 +  }
 +
 +  /**
-    * Answers a 'namespace' prefix to use for features created in Jalview from
-    * attributes in the structure viewer
-    * 
-    * @return
-    */
-   protected String getStructureFeaturePrefix()
-   {
-     // TODO pull up as abstract
-     return CHIMERA_FEATURE_PREFIX;
-   }
- 
-   /**
 +   * Answers the feature group name to apply to features created in Jalview from
 +   * Chimera attributes
 +   * 
 +   * @return
 +   */
 +  protected String getViewerFeatureGroup()
 +  {
 +    // todo pull up to interface
 +    return CHIMERA_FEATURE_GROUP;
 +  }
++
++
+   public Hashtable<String, String> getChainFile()
+   {
+     return chainFile;
+   }
+ 
+   public List<ChimeraModel> getChimeraModelByChain(String chain)
+   {
+     return chimeraMaps.get(chainFile.get(chain));
+   }
+ 
+   public int getModelNoForChain(String chain)
+   {
+     List<ChimeraModel> foundModels = getChimeraModelByChain(chain);
+     if (foundModels != null && !foundModels.isEmpty())
+     {
+       return foundModels.get(0).getModelNumber();
+     }
+     return -1;
+   }
  }
diff --cc src/jalview/gui/ChimeraViewFrame.java
index d43702c,fe12f40..e76f11e
--- a/src/jalview/gui/ChimeraViewFrame.java
+++ b/src/jalview/gui/ChimeraViewFrame.java
@@@ -342,11 -246,9 +343,8 @@@ public class ChimeraViewFrame extends S
            SequenceI[][] seqs)
    {
      createProgressBar();
--    // FIXME extractChains needs pdbentries to match IDs to PDBEntry(s) on seqs
-     String[][] chains = extractChains(seqs);
      jmb = new JalviewChimeraBindingModel(this,
-             ap.getStructureSelectionManager(), pdbentrys, seqs, chains,
-             null);
+             ap.getStructureSelectionManager(), pdbentrys, seqs, null);
      addAlignmentPanel(ap);
      useAlignmentPanelForColourbyseq(ap);
      if (pdbentrys.length > 1)
@@@ -373,43 -275,9 +371,7 @@@
  
    }
  
 -
 -
    /**
-    * Retrieve chains for sequences by inspecting their PDB refs. The hope is
-    * that the first will be to the sequence's own chain. Really need a more
-    * managed way of doing this.
-    * 
-    * @param seqs
-    * @return
-    */
-   protected String[][] extractChains(SequenceI[][] seqs)
-   {
-     String[][] chains = new String[seqs.length][];
-     for (int i = 0; i < seqs.length; i++)
-     {
-       chains[i] = new String[seqs[i].length];
-       int seqno = 0;
-       for (SequenceI seq : seqs[i])
-       {
-         String chain = null;
-         if (seq.getDatasetSequence() != null)
-         {
-           Vector<PDBEntry> pdbrefs = seq.getDatasetSequence()
-                   .getAllPDBEntries();
-           if (pdbrefs != null && pdbrefs.size() > 0)
-           {
-             // FIXME: SequenceI.PDBEntry[0] chain mapping used for
-             // ChimeraViewFrame. Is this even used ???
- 
-             chain = pdbrefs.get(0).getChainCode();
-           }
-         }
-         chains[i][seqno++] = chain;
-       }
-     }
-     return chains;
-   }
- 
-   /**
     * Create a new viewer from saved session state data including Chimera session
     * file
     * 
@@@ -502,9 -370,10 +464,8 @@@
     */
    void initChimera()
    {
-     jalview.gui.Desktop.addInternalFrame(this,
-             jmb.getViewerTitle("Chimera", true), getBounds().width,
-             getBounds().height);
 -    jmb.setFinishedInit(false);
 -    jalview.gui.Desktop.addInternalFrame(this,
 -            jmb.getViewerTitle("Chimera", true), getBounds().width,
 -            getBounds().height);
++    Desktop.addInternalFrame(this, jmb.getViewerTitle("Chimera", true),
++            getBounds().width, getBounds().height);
  
      if (!jmb.launchChimera())
      {
@@@ -530,59 -400,6 +491,60 @@@
      jmb.startChimeraListener();
    }
  
 +  /**
 +   * If the list is not empty, add menu items for 'All' and each individual
 +   * chain to the "View | Show Chain" sub-menu. Multiple selections are allowed.
 +   * 
 +   * @param chainNames
 +   */
++  @Override
 +  void setChainMenuItems(List<String> chainNames)
 +  {
 +    chainMenu.removeAll();
 +    if (chainNames == null || chainNames.isEmpty())
 +    {
 +      return;
 +    }
 +    JMenuItem menuItem = new JMenuItem(
 +            MessageManager.getString("label.all"));
 +    menuItem.addActionListener(new ActionListener()
 +    {
 +      @Override
 +      public void actionPerformed(ActionEvent evt)
 +      {
 +        allChainsSelected = true;
 +        for (int i = 0; i < chainMenu.getItemCount(); i++)
 +        {
 +          if (chainMenu.getItem(i) instanceof JCheckBoxMenuItem)
 +          {
 +            ((JCheckBoxMenuItem) chainMenu.getItem(i)).setSelected(true);
 +          }
 +        }
 +        showSelectedChains();
 +        allChainsSelected = false;
 +      }
 +    });
 +
 +    chainMenu.add(menuItem);
 +
 +    for (String chainName : chainNames)
 +    {
 +      menuItem = new JCheckBoxMenuItem(chainName, true);
 +      menuItem.addItemListener(new ItemListener()
 +      {
 +        @Override
 +        public void itemStateChanged(ItemEvent evt)
 +        {
 +          if (!allChainsSelected)
 +          {
 +            showSelectedChains();
 +          }
 +        }
 +      });
 +
 +      chainMenu.add(menuItem);
 +    }
 +  }
  
    /**
     * Show only the selected chain(s) in the viewer
@@@ -789,7 -607,7 +753,8 @@@
            }
          }
        }
 +
+       jmb.refreshGUI();
        jmb.setFinishedInit(true);
        jmb.setLoadingFromArchive(false);
  
diff --cc test/jalview/datamodel/SequenceFeatureTest.java
index 2ec824d,2ec824d..5150337
--- a/test/jalview/datamodel/SequenceFeatureTest.java
+++ b/test/jalview/datamodel/SequenceFeatureTest.java
@@@ -112,56 -112,56 +112,83 @@@ public class SequenceFeatureTes
      assertEquals(sf1.hashCode(), sf2.hashCode());
  
      // changing type breaks equals:
++    String restores = sf2.getType();
      sf2.setType("Type");
      assertFalse(sf1.equals(sf2));
++    sf2.setType(restores);
  
      // changing description breaks equals:
--    sf2.setType("type");
++    restores = sf2.getDescription();
      sf2.setDescription("Desc");
      assertFalse(sf1.equals(sf2));
++    sf2.setDescription(restores);
++
++    // changing score breaks equals:
++    float restoref = sf2.getScore();
++    sf2.setScore(12.4f);
++    assertFalse(sf1.equals(sf2));
++    sf2.setScore(restoref);
++
++    // NaN doesn't match a number
++    restoref = sf2.getScore();
++    sf2.setScore(Float.NaN);
++    assertFalse(sf1.equals(sf2));
++
++    // NaN matches NaN
++    sf1.setScore(Float.NaN);
++    assertTrue(sf1.equals(sf2));
++    sf1.setScore(restoref);
++    sf2.setScore(restoref);
  
      // changing start position breaks equals:
--    sf2.setDescription("desc");
++    int restorei = sf2.getBegin();
      sf2.setBegin(21);
      assertFalse(sf1.equals(sf2));
++    sf2.setBegin(restorei);
  
      // changing end position breaks equals:
--    sf2.setBegin(22);
++    restorei = sf2.getEnd();
      sf2.setEnd(32);
      assertFalse(sf1.equals(sf2));
++    sf2.setEnd(restorei);
  
      // changing feature group breaks equals:
--    sf2.setEnd(33);
++    restores = sf2.getFeatureGroup();
      sf2.setFeatureGroup("Group");
      assertFalse(sf1.equals(sf2));
++    sf2.setFeatureGroup(restores);
  
      // changing ID breaks equals:
--    sf2.setFeatureGroup("group");
++    restores = (String) sf2.getValue("ID");
      sf2.setValue("ID", "id2");
      assertFalse(sf1.equals(sf2));
++    sf2.setValue("ID", restores);
  
      // changing Name breaks equals:
--    sf2.setValue("ID", "id");
++    restores = (String) sf2.getValue("Name");
      sf2.setValue("Name", "Name");
      assertFalse(sf1.equals(sf2));
++    sf2.setValue("Name", restores);
  
      // changing Parent breaks equals:
--    sf2.setValue("Name", "name");
++    restores = (String) sf1.getValue("Parent");
      sf1.setValue("Parent", "Parent");
      assertFalse(sf1.equals(sf2));
++    sf1.setValue("Parent", restores);
  
      // changing strand breaks equals:
--    sf1.setValue("Parent", "parent");
++    restorei = sf2.getStrand();
      sf2.setStrand("-");
      assertFalse(sf1.equals(sf2));
++    sf2.setStrand(restorei == 1 ? "+" : "-");
  
      // changing phase breaks equals:
--    sf2.setStrand("+");
++    restores = sf1.getPhase();
      sf1.setPhase("2");
      assertFalse(sf1.equals(sf2));
++    sf1.setPhase(restores);
  
      // restore equality as sanity check:
--    sf1.setPhase("1");
      assertTrue(sf1.equals(sf2));
      assertTrue(sf2.equals(sf1));
      assertEquals(sf1.hashCode(), sf2.hashCode());