fix for MDLV2000Writer when SGroup Parent Atoms (SPA) is a List instead of a Set

dkatzel-ncats · dkatzel-ncats · commit 3d5fdeb07a6c · 2020-09-11T15:36:05.000-04:00
diff --git a/display/renderbasic/src/main/java/org/openscience/cdk/renderer/generators/standard/StandardSgroupGenerator.java b/display/renderbasic/src/main/java/org/openscience/cdk/renderer/generators/standard/StandardSgroupGenerator.java
@@ -206,7 +206,7 @@ private static void hideMultipleParts(IAtomContainer container, Sgroup sgroup) {
 
         final Set<IBond> crossing = sgroup.getBonds();
         final Set<IAtom> atoms = sgroup.getAtoms();
-        final Set<IAtom> parentAtoms = sgroup.getValue(SgroupKey.CtabParentAtomList);
+        final Collection<IAtom> parentAtoms = sgroup.getValue(SgroupKey.CtabParentAtomList);
 
         for (IBond bond : container.bonds()) {
             if (parentAtoms.contains(bond.getBegin()) && parentAtoms.contains(bond.getEnd()))
diff --git a/storage/ctab/src/main/java/org/openscience/cdk/io/MDLV2000Reader.java b/storage/ctab/src/main/java/org/openscience/cdk/io/MDLV2000Reader.java
@@ -73,15 +73,7 @@
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.io.StringReader;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.StringTokenizer;
+import java.util.*;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -1256,7 +1248,7 @@ void readPropertiesFast(final BufferedReader input, final IAtomContainer contain
                 case M_SPA:
                     sgroup = ensureSgroup(sgroups, readMolfileInt(line, 7));
                     count  = readMolfileInt(line, 10);
-                    Set<IAtom> parentAtomList = sgroup.getValue(SgroupKey.CtabParentAtomList);
+                    Collection<IAtom> parentAtomList = sgroup.getValue(SgroupKey.CtabParentAtomList);
                     if (parentAtomList == null) {
                         sgroup.putValue(SgroupKey.CtabParentAtomList, parentAtomList = new HashSet<IAtom>());
                     }
diff --git a/storage/ctab/src/main/java/org/openscience/cdk/io/MDLV2000Writer.java b/storage/ctab/src/main/java/org/openscience/cdk/io/MDLV2000Writer.java
@@ -1030,7 +1030,7 @@ private void writeSgroups(IAtomContainer container, BufferedWriter writer, Map<I
                         writer.write('\n');
                         break;
                     case CtabParentAtomList:
-                        Set<IAtom> parentAtomList = sgroup.getValue(key);
+                        Collection<IAtom> parentAtomList = sgroup.getValue(key);
                         for (List<IAtom> atoms : wrap(parentAtomList, 15)) {
                             writer.write("M  SPA ");
                             writer.write(formatMDLInt(id, 3));
diff --git a/storage/ctab/src/main/java/org/openscience/cdk/io/iterator/IteratingSDFReader.java b/storage/ctab/src/main/java/org/openscience/cdk/io/iterator/IteratingSDFReader.java
@@ -259,6 +259,7 @@ public boolean hasNext() {
                         reader.setReader(new StringReader(buffer.toString()));
                         molecule = reader.read(builder.newAtomContainer());
                     } catch (Exception exception) {
+                        exception.printStackTrace();
                         logger.error("Error while reading next molecule: " + exception.getMessage());
                         logger.debug(exception);
                     }
diff --git a/storage/ctab/src/test/java/org/openscience/cdk/io/MDLV2000WriterTest.java b/storage/ctab/src/test/java/org/openscience/cdk/io/MDLV2000WriterTest.java
@@ -47,14 +47,19 @@
 import org.openscience.cdk.interfaces.ITetrahedralChirality;
 import org.openscience.cdk.io.listener.PropertiesListener;
 import org.openscience.cdk.sgroup.Sgroup;
+import org.openscience.cdk.sgroup.SgroupKey;
+import org.openscience.cdk.sgroup.SgroupType;
+import org.openscience.cdk.silent.SilentChemObjectBuilder;
 import org.openscience.cdk.templates.TestMoleculeFactory;
 
 import javax.vecmath.Point2d;
 import javax.vecmath.Point3d;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.StringWriter;
+import java.util.ArrayList;
 import java.util.Collections;
+import java.util.List;
 import java.util.Properties;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -971,4 +976,29 @@ public void skipDefaultProps() throws Exception {
                                               + "M  END"));
         }
     }
+
+    @Test
+    public void writeParentAtomSgroupAsList() throws Exception{
+        IAtomContainer mol  = builder.newAtomContainer();
+        IAtom          atom = builder.newAtom();
+        atom.setSymbol("C");
+        mol.addAtom(atom);
+        // build multiple group Sgroup
+        Sgroup sgroup = new Sgroup();
+        sgroup.setType(SgroupType.CtabMultipleGroup);
+        sgroup.addAtom(atom);
+        List<IAtom> patoms = new ArrayList<>();
+
+            patoms.add(atom);
+
+        sgroup.putValue(SgroupKey.CtabParentAtomList, patoms);
+        mol.setProperty(CDKConstants.CTAB_SGROUPS,
+                Collections.singletonList(sgroup));
+        StringWriter sw = new StringWriter();
+        try (MDLV2000Writer mdlw = new MDLV2000Writer(sw)) {
+            mdlw.write(mol);
+        }
+        assertThat(sw.toString(), containsString("SPA   1  1"));
+
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -259,6 +259,7 @@ public boolean hasNext() {`
`259`	`259`	`reader.setReader(new StringReader(buffer.toString()));`
`260`	`260`	`molecule = reader.read(builder.newAtomContainer());`
`261`	`261`	`} catch (Exception exception) {`
	`262`	`+ exception.printStackTrace();`
`262`	`263`	`logger.error("Error while reading next molecule: " + exception.getMessage());`
`263`	`264`	`logger.debug(exception);`
`264`	`265`	`}`