Include unpaired electrons sooner to catch negative hydrogen count in…

… existing conditionals [Fixes:1343]. Signed-off-by: Egon Willighagen <egonw@users.sourceforge.net>
cdk · Jul 15, 2014 · 9b9791d · 9b9791d
1 parent d7f8a2c
commit 9b9791d
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 5 deletions.
diff --git a/base/testdata/src/test/resources/data/mdl/ChEBI_29293.mol b/base/testdata/src/test/resources/data/mdl/ChEBI_29293.mol
@@ -0,0 +1,10 @@
+
+  Marvin  10300612312D          
+
+  2  1  0  0  0  0            999 V2000
+   -0.4125    0.0000    0.0000 H   0  0  0  0  0  0  0  0  0  0  0  0
+    0.4125    0.0000    0.0000 H   0  3  0  0  0  0  0  0  0  0  0  0
+  2  1  1  0  0  0  0
+M  CHG  1   2   1
+M  RAD  1   1   2
+M  END
diff --git a/storage/io/src/main/java/org/openscience/cdk/io/MDLV2000Reader.java b/storage/io/src/main/java/org/openscience/cdk/io/MDLV2000Reader.java
@@ -466,9 +466,9 @@ else if (!hasZ) {
                 if (valence < 0) {
                     hasQueryBonds = true; // also counts aromatic bond as query
                 } else {
+                    int unpaired = outputContainer.getConnectedSingleElectronsCount(outputContainer.getAtom(i));
                     applyMDLValenceModel(outputContainer.getAtom(i),
-                                         valence,
-                                         outputContainer.getConnectedSingleElectronsCount(outputContainer.getAtom(i)));
+                                         valence + unpaired);
                 }
             }
 
@@ -512,11 +512,11 @@ else if (!hasZ) {
      * @param atom            the atom to apply the model to
      * @param explicitValence the explicit valence (bond order sum)
      */
-    private void applyMDLValenceModel(IAtom atom, int explicitValence, int unpaired) {
+    private void applyMDLValenceModel(IAtom atom, int explicitValence) {
 
         if (atom.getValency() != null) {
             if (atom.getValency() >= explicitValence)
-                atom.setImplicitHydrogenCount(atom.getValency() - explicitValence - unpaired);
+                atom.setImplicitHydrogenCount(atom.getValency() - explicitValence);
             else
                 atom.setImplicitHydrogenCount(0);
         }
@@ -536,7 +536,7 @@ private void applyMDLValenceModel(IAtom atom, int explicitValence, int unpaired)
             }
             else {
                 atom.setValency(implicitValence);
-                atom.setImplicitHydrogenCount(implicitValence - explicitValence - unpaired);
+                atom.setImplicitHydrogenCount(implicitValence - explicitValence);
             }
         }
     }

diff --git a/storage/io/src/test/java/org/openscience/cdk/io/MDLV2000ReaderTest.java b/storage/io/src/test/java/org/openscience/cdk/io/MDLV2000ReaderTest.java
@@ -1355,4 +1355,20 @@ public void testMultipleRadicals() throws Exception {
                        is(notNullValue()));
         }
     }
+
+    /**
+     * @cdk.bug 1343
+     */
+    @Test public void nonNegativeHydrogenCountOnHydrogenRadical() throws Exception {
+        InputStream in = getClass().getResourceAsStream("/data/mdl/ChEBI_29293.mol");
+        MDLV2000Reader reader = new MDLV2000Reader(in);
+        IAtomContainer container = reader.read(new AtomContainer());
+        reader.close();
+        assertThat(container.getAtom(0).getImplicitHydrogenCount(),
+                   is(0));
+        assertThat(container.getAtom(1).getImplicitHydrogenCount(),
+                   is(0));
+
+
+    }
 }