A corner case of suppress hydrogens, we may have a double bond define…

…d with an explicit hydrogen and need to delete the stereo there. Also the new isotope logic means we should keep H when the mass is specified as 1.
cdk · Nov 7, 2017 · e844764 · e844764
1 parent e8d7417
commit e844764
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 6 deletions.
diff --git a/...tandard/src/main/java/org/openscience/cdk/tools/manipulator/AtomContainerManipulator.java b/...tandard/src/main/java/org/openscience/cdk/tools/manipulator/AtomContainerManipulator.java
@@ -838,8 +838,10 @@ public static IAtomContainer suppressHydrogens(IAtomContainer org) {
                     yNew = findOther(org, v, u, y);
                 }
 
-                // no other atoms connected, invalid double-bond configuration?
-                if (x == null || y == null) continue;
+                // no other atoms connected, invalid double-bond configuration
+                // is removed. example [2H]/C=C/[H]
+                if (x == null || y == null ||
+                    xNew == null || yNew == null) continue;
 
                 // no changes
                 if (x.equals(xNew) && y.equals(yNew)) {
@@ -848,10 +850,12 @@ public static IAtomContainer suppressHydrogens(IAtomContainer org) {
                 }
 
                 // XXX: may perform slow operations but works for now
-                IBond cpyLeft = !Objects.equals(xNew, x) ? org.getBond(u, xNew) : orgLeft;
+                IBond cpyLeft  = !Objects.equals(xNew, x) ? org.getBond(u, xNew) : orgLeft;
                 IBond cpyRight = !Objects.equals(yNew, y) ? org.getBond(v, yNew) : orgRight;
 
-                elements.add(new DoubleBondStereochemistry(orgStereo, new IBond[]{cpyLeft, cpyRight}, conformation));
+                elements.add(new DoubleBondStereochemistry(orgStereo,
+                                                           new IBond[]{cpyLeft, cpyRight},
+                                                           conformation));
             } else if (se instanceof Atropisomeric) {
                 // can not have any H's
                 elements.add(se);
@@ -917,7 +921,7 @@ private static boolean suppressibleHydrogen(final IAtomContainer container, fina
         // is the hydrogen an ion?
         if (atom.getFormalCharge() != null && atom.getFormalCharge() != 0) return false;
         // is the hydrogen deuterium / tritium?
-        if (atom.getMassNumber() != null && atom.getMassNumber() != 1) return false;
+        if (atom.getMassNumber() != null) return false;
         // molecule hydrogen with implicit H?
         if (atom.getImplicitHydrogenCount() != null && atom.getImplicitHydrogenCount() != 0) return false;
         // molecule hydrogen
@@ -968,7 +972,7 @@ private static boolean suppressibleHydrogen(final IAtomContainer container, fina
         // is the hydrogen an ion?
         if (atom.getFormalCharge() != null && atom.getFormalCharge() != 0) return false;
         // is the hydrogen deuterium / tritium?
-        if (atom.getMassNumber() != null && atom.getMassNumber() != 1) return false;
+        if (atom.getMassNumber() != null) return false;
         // hydrogen is either not attached to 0 or 2 neighbors
         if (graph[v].length != 1) return false;
         // non-single bond

diff --git a/...ard/src/test/java/org/openscience/cdk/tools/manipulator/AtomContainerManipulatorTest.java b/...ard/src/test/java/org/openscience/cdk/tools/manipulator/AtomContainerManipulatorTest.java
@@ -33,6 +33,7 @@
 import java.util.List;
 import java.util.Map;
 
+import org.hamcrest.CoreMatchers;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -1279,6 +1280,23 @@ public void molecularWeight() throws InvalidSmilesException, IOException {
         assertThat(exactMass, closeTo(48.053, 0.001));
     }
 
+    @Test
+    public void removeBondStereo() throws Exception {
+        SmilesParser smipar = new SmilesParser(SilentChemObjectBuilder.getInstance());
+        IAtomContainer mol = smipar.parseSmiles("[2H]/C=C/[H]");
+        AtomContainerManipulator.suppressHydrogens(mol);
+        assertThat(mol.stereoElements().iterator().hasNext(),
+                   CoreMatchers.is(false));
+    }
+
+    @Test
+    public void keep1Hisotopes() throws Exception {
+        SmilesParser smipar = new SmilesParser(SilentChemObjectBuilder.getInstance());
+        IAtomContainer mol = smipar.parseSmiles("[2H]/C=C/[1H]");
+        AtomContainerManipulator.suppressHydrogens(mol);
+        assertThat(mol.getAtomCount(), is(4));
+    }
+
     // util for testing hydrogen removal using SMILES
     static void assertRemoveH(String smiIn, String smiExp) throws Exception {
         SmilesParser smipar = new SmilesParser(SilentChemObjectBuilder.getInstance());