More compact writing Sgroup hierarchy/nesting info. We can compact things but collecting up all the parents and then writing them all at once.

johnmay · johnmay · commit 907c2bd75993 · 2020-09-29T11:52:29.000+01:00
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
@@ -66,7 +66,18 @@
 import java.nio.charset.StandardCharsets;
 import java.text.NumberFormat;
 import java.text.SimpleDateFormat;
-import java.util.*;
+import java.util.AbstractMap;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -978,9 +989,18 @@ private void writeSgroups(IAtomContainer container, BufferedWriter writer, Map<I
         // going to modify
         sgroups = new ArrayList<>(sgroups);
 
+
         // remove non-ctab Sgroups
         sgroups.removeIf(sgroup -> !sgroup.getType().isCtabStandard());
 
+        List<Map.Entry<Sgroup,Sgroup>> parentList = new ArrayList<>();
+
+        // collect parents
+        for (Sgroup sgroup : sgroups) {
+            for (Sgroup parent : sgroup.getParents())
+                parentList.add(new AbstractMap.SimpleEntry<>(sgroup, parent));
+        }
+
         for (List<Sgroup> wrapSgroups : wrap(sgroups, 8)) {
             // Declare the SGroup type
             writer.write("M  STY");
@@ -994,6 +1014,19 @@ private void writeSgroups(IAtomContainer container, BufferedWriter writer, Map<I
             writer.write('\n');
         }
 
+        // Sgroup Parent List
+        for (List<Map.Entry<Sgroup,Sgroup>> parents : wrap(parentList, 8)) {
+            writer.write("M  SPL");
+            writer.write(formatMDLInt(parents.size(), 3));
+            for (Map.Entry<Sgroup,Sgroup> e : parents) {
+                writer.write(' ');
+                writer.write(formatMDLInt(1+sgroups.indexOf(e.getKey()), 3));
+                writer.write(' ');
+                writer.write(formatMDLInt(1+sgroups.indexOf(e.getValue()), 3));
+            }
+            writer.write('\n');
+        }
+
         // Sgroup output is non-compact for now - but valid
         for (int id = 1; id <= sgroups.size(); id++) {
             Sgroup sgroup = sgroups.get(id - 1);
@@ -1022,19 +1055,6 @@ private void writeSgroups(IAtomContainer container, BufferedWriter writer, Map<I
                 writer.write('\n');
             }
 
-            // Sgroup Parent List
-            for (List<Sgroup> parents : wrap(sgroup.getParents(), 8)) {
-                writer.write("M  SPL");
-                writer.write(formatMDLInt(parents.size(), 3));
-                for (Sgroup parent : parents) {
-                    writer.write(' ');
-                    writer.write(formatMDLInt(id, 3));
-                    writer.write(' ');
-                    writer.write(formatMDLInt(1 + sgroups.indexOf(parent), 3));
-                }
-                writer.write('\n');
-            }
-
             Set<SgroupKey> attributeKeys = sgroup.getAttributeKeys();
             // TODO order and aggregate attribute keys
             for (SgroupKey key : attributeKeys) {
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
@@ -719,7 +719,7 @@ public void sgroupUnorderedMixtureRoundTrip() throws Exception {
             mdlw.write(mdlr.read(new AtomContainer()));
             String output = sw.toString();
             assertThat(output, containsString("M  STY  3   1 COM   2 COM   3 MIX"));
-            assertThat(output, containsString("M  SPL  1   1   3"));
+            assertThat(output, containsString("M  SPL  2   1   3   2   3"));
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -719,7 +719,7 @@ public void sgroupUnorderedMixtureRoundTrip() throws Exception {`
`719`	`719`	`mdlw.write(mdlr.read(new AtomContainer()));`
`720`	`720`	`String output = sw.toString();`
`721`	`721`	`assertThat(output, containsString("M STY 3 1 COM 2 COM 3 MIX"));`
`722`		`- assertThat(output, containsString("M SPL 1 1 3"));`
	`722`	`+ assertThat(output, containsString("M SPL 2 1 3 2 3"));`
`723`	`723`	`}`
`724`	`724`	`}`
`725`	`725`