CXSMILES corner case - collect the reaction atom index before merging the fragments together.

johnmay · johnmay · commit ebbf42ddd717 · 2021-04-01T20:52:44.000+01:00
diff --git a/storage/smiles/src/main/java/org/openscience/cdk/smiles/SmilesParser.java b/storage/smiles/src/main/java/org/openscience/cdk/smiles/SmilesParser.java
@@ -346,26 +346,32 @@ private void parseRxnCXSMILES(String title, IReaction rxn) throws InvalidSmilesE
 
                 final Map<IAtom, IAtomContainer> atomToMol = new HashMap<>(100);
                 final List<IAtom> atoms = new ArrayList<>();
+
+                // collect atom offsets before handling fragment groups
+                for (IAtomContainer mol : rxn.getReactants().atomContainers())
+                    for (IAtom atom : mol.atoms())
+                        atoms.add(atom);
+                for (IAtomContainer mol : rxn.getAgents().atomContainers())
+                    for (IAtom atom : mol.atoms())
+                        atoms.add(atom);
+                for (IAtomContainer mol : rxn.getProducts().atomContainers())
+                    for (IAtom atom : mol.atoms())
+                        atoms.add(atom);
+
                 handleFragmentGrouping(rxn, cxstate);
 
                 // merge all together
                 for (IAtomContainer mol : rxn.getReactants().atomContainers()) {
-                    for (IAtom atom : mol.atoms()) {
-                        atoms.add(atom);
+                    for (IAtom atom : mol.atoms())
                         atomToMol.put(atom, mol);
-                    }
                 }
                 for (IAtomContainer mol : rxn.getAgents().atomContainers()) {
-                    for (IAtom atom : mol.atoms()) {
-                        atoms.add(atom);
+                    for (IAtom atom : mol.atoms())
                         atomToMol.put(atom, mol);
-                    }
                 }
                 for (IAtomContainer mol : rxn.getProducts().atomContainers()) {
-                    for (IAtom atom : mol.atoms()) {
-                        atoms.add(atom);
+                    for (IAtom atom : mol.atoms())
                         atomToMol.put(atom, mol);
-                    }
                 }
 
                 assignCxSmilesInfo(rxn.getBuilder(), rxn, atoms, atomToMol, cxstate);
diff --git a/storage/smiles/src/test/java/org/openscience/cdk/smiles/CxSmilesParserTest.java b/storage/smiles/src/test/java/org/openscience/cdk/smiles/CxSmilesParserTest.java
@@ -26,19 +26,24 @@
 import org.hamcrest.BaseMatcher;
 import org.hamcrest.Description;
 import org.junit.Test;
+import org.openscience.cdk.exception.CDKException;
 import org.openscience.cdk.exception.InvalidSmilesException;
 import org.openscience.cdk.interfaces.IAtom;
 import org.openscience.cdk.interfaces.IAtomContainer;
+import org.openscience.cdk.interfaces.IPseudoAtom;
 import org.openscience.cdk.interfaces.IReaction;
 import org.openscience.cdk.interfaces.IStereoElement;
 import org.openscience.cdk.silent.SilentChemObjectBuilder;
+import org.openscience.cdk.tools.manipulator.ReactionManipulator;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import static org.hamcrest.CoreMatchers.hasItem;
+import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -354,7 +359,16 @@ public void hydrogenAndCoordinationBondingSkipped() {
         }
     }
 
-
+    @Test public void atomOrderingWithNonContiguousFragments() throws CDKException {
+        SmilesParser smipar = new SmilesParser(SilentChemObjectBuilder.getInstance());
+        IReaction rxn = smipar.parseReactionSmiles("C.*.C>> |$;R1;$,f:0.2|");
+        List<IAtomContainer> mols = ReactionManipulator.getAllAtomContainers(rxn);
+        assertThat(mols.get(0).getAtomCount(), is(2));
+        assertThat(mols.get(1).getAtomCount(), is(1));
+        assertThat(mols.get(1).getAtom(0), is(instanceOf(IPseudoAtom.class)));
+        assertThat(((IPseudoAtom)mols.get(1).getAtom(0)).getLabel(),
+                is("R1"));
+    }
 
     /**
      * Custom matcher for checking an array of doubles closely matches (epsilon=0.01)
