Correctly replace atom in MDLV3000Reader. Setting just the atom does …

…not update the bonds that reference that atom. Change-Id: Ib52bf59b365fd927208e6c54644acef19599179a Signed-off-by: Egon Willighagen <egonw@users.sourceforge.net>
cdk · Nov 9, 2014 · b96674b · b96674b
1 parent ce5ae5d
commit b96674b
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 5 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
@@ -162,6 +162,7 @@ public static boolean replaceAtomByAtom(IAtomContainer container, IAtom atom, IA
             }
             return true;
         }
+        // FIXME - also need to handle stereochemistry
     }
 
     /**

diff --git a/storage/io/src/main/java/org/openscience/cdk/io/MDLV3000Reader.java b/storage/io/src/main/java/org/openscience/cdk/io/MDLV3000Reader.java
@@ -50,6 +50,7 @@
 import org.openscience.cdk.io.formats.MDLV3000Format;
 import org.openscience.cdk.tools.ILoggingTool;
 import org.openscience.cdk.tools.LoggingToolFactory;
+import org.openscience.cdk.tools.manipulator.AtomContainerManipulator;
 import org.openscience.cdk.tools.manipulator.BondManipulator;
 
 /**
@@ -491,12 +492,15 @@ public void readSGroup(IAtomContainer readData) throws CDKException {
                             throw new CDKException(error, exception);
                         }
                         if (atomID != -1 && label.length() > 0) {
-                            IAtom atom = readData.getAtom(atomID - 1);
-                            if (!(atom instanceof IPseudoAtom)) {
-                                atom = readData.getBuilder().newInstance(IPseudoAtom.class, atom);
+                            IAtom original = readData.getAtom(atomID - 1);
+                            IAtom replacement = original;
+                            if (!(original instanceof IPseudoAtom)) {
+                                replacement = readData.getBuilder().newInstance(IPseudoAtom.class,
+                                                                                original);
                             }
-                            ((IPseudoAtom) atom).setLabel(label);
-                            readData.setAtom(atomID - 1, atom);
+                            ((IPseudoAtom) replacement).setLabel(label);
+                            if (replacement != original)
+                                AtomContainerManipulator.replaceAtomByAtom(readData, original, replacement);
                         }
                     }
                 } else {

diff --git a/storage/io/src/test/java/org/openscience/cdk/io/MDLV3000ReaderTest.java b/storage/io/src/test/java/org/openscience/cdk/io/MDLV3000ReaderTest.java
@@ -27,6 +27,7 @@
 import org.junit.Test;
 import org.openscience.cdk.DefaultChemObjectBuilder;
 import org.openscience.cdk.exception.CDKException;
+import org.openscience.cdk.graph.GraphUtil;
 import org.openscience.cdk.interfaces.IAtom;
 import org.openscience.cdk.interfaces.IAtomContainer;
 import org.openscience.cdk.silent.AtomContainer;
@@ -107,5 +108,13 @@ public void testPseudoAtomLabels() throws Exception {
         IPseudoAtom pa = (IPseudoAtom) molecule.getAtom(9);
         Assert.assertEquals("Leu", pa.getLabel());
     }
+
+    @Test public void pseudoAtomReplacement() throws Exception {
+        MDLV3000Reader reader = new MDLV3000Reader(getClass().getResourceAsStream("pseudoAtomReplacement.mol"));
+        IAtomContainer container = reader.read(new org.openscience.cdk.AtomContainer(0,0,0,0));
+        for (IAtom atom : container.getBond(9).atoms()) {
+            Assert.assertTrue(container.contains(atom));
+        }
+    }
 
 }
diff --git a/storage/io/src/test/resources/org/openscience/cdk/io/pseudoAtomReplacement.mol b/storage/io/src/test/resources/org/openscience/cdk/io/pseudoAtomReplacement.mol
@@ -0,0 +1,49 @@
+
+ -ISIS-  10171405052D
+
+ 0  0  0     0  0              0 V3000
+M  V30 BEGIN CTAB
+M  V30 COUNTS 14 15 1 0 0
+M  V30 BEGIN ATOM
+M  V30 1 C 6.4292 -1.1916 0 0 CFG=3
+M  V30 2 C 7.0125 -0.6042 0 0
+M  V30 3 N 6.4292 -0.0250999 0 0
+M  V30 4 C 5.8416 -0.6042 0 0
+M  V30 5 C 5.8416 -1.7708 0 0
+M  V30 6 N 6.4292 -2.3584 0 0 CFG=3
+M  V30 7 C 7.0125 -1.7708 0 0
+M  V30 8 O 5.7166 -3.5875 0 0
+M  V30 9 C 5.7166 -4.4125 0 0 CFG=3
+M  V30 10 C 4.8875 -4.4125 0 0
+M  V30 11 C 6.5376 -4.4166 0 0
+M  V30 12 C 5.7166 -5.2376 0 0
+M  V30 13 C 6.4292 -3.175 0 0
+M  V30 14 O 7.1375 -3.5875 0 0
+M  V30 END ATOM
+M  V30 BEGIN BOND
+M  V30 1 1 1 2
+M  V30 2 1 2 3
+M  V30 3 1 3 4
+M  V30 4 1 4 1
+M  V30 5 1 1 5
+M  V30 6 1 5 6
+M  V30 7 1 6 7
+M  V30 8 1 7 1
+M  V30 9 1 6 13
+M  V30 10 1 8 9
+M  V30 11 1 9 10
+M  V30 12 1 9 11
+M  V30 13 1 9 12
+M  V30 14 2 13 14
+M  V30 15 1 8 13
+M  V30 END BOND
+M  V30 BEGIN SGROUP
+M  V30 1 SUP 0 ATOMS=(7 8 9 10 11 12 13 14) XBONDS=(1 9) BRKXYZ=(9 6.24 -2.9 0 -
+M  V30 6.24 -2.9 0 0 0 0) CSTATE=(4 9 0 0.82 0) LABEL=Boc
+M  V30 END SGROUP
+M  V30 BEGIN COLLECTION
+M  V30 MDLV30/HILITE ATOMS=(14 1 2 3 4 5 6 7 8 9 10 11 12 13 14) BONDS=(15 1 2 -
+M  V30 3 4 5 6 7 8 9 10 11 12 13 14 15) SGROUPS=(1 1)
+M  V30 END COLLECTION
+M  V30 END CTAB
+M  END