From acdc158f5c20ab04bb40e3b0580f3da02e4f744e Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Wed, 16 Nov 2016 10:53:03 +0000
Subject: [PATCH] JAL-2296 don't copy attribute with value False

---
 .../ext/rbvi/chimera/JalviewChimeraBinding.java    |   41 +++++++++++++-------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
index 54adf87..18a614d 100644
--- a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
+++ b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
@@ -1236,6 +1236,9 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
             .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;
     }
 
@@ -1253,6 +1256,8 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
      * and then traverse it to find feature ranges.
      */
     boolean featureAdded = false;
+    String featureGroup = getViewerFeatureGroup();
+
     for (String residue : residues)
     {
       AtomSpec spec = null;
@@ -1263,10 +1268,16 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
       }
       String atomSpec = tokens[2];
       String attValue = tokens[4];
-      if ("None".equals(attValue))
+
+      /*
+       * 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);
@@ -1275,6 +1286,19 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
         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);
 
@@ -1291,18 +1315,9 @@ public abstract class JalviewChimeraBinding extends AAStructureBindingModel
         SequenceI seq = m.getSequence();
         int start = m.getStart();
         int end = m.getEnd();
-
-        float score = Float.NaN;
-        try
-        {
-          score = Float.valueOf(attValue);
-        } catch (NumberFormatException e)
-        {
-          // was not a float value
-        }
-        String featureGroup = getViewerFeatureGroup();
-        SequenceFeature sf = new SequenceFeature(attName, attValue, start,
-                end, score, featureGroup);
+        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);
       }
-- 
1.7.10.2