Hydrogens can still be deleted from Sgroups, so we need to update the…

… sgroups accordingly.
cdk · Jan 13, 2018 · 90b6bc3 · 90b6bc3
1 parent df5da54
commit 90b6bc3
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 8 deletions.
diff --git a/base/core/src/main/java/org/openscience/cdk/sgroup/Sgroup.java b/base/core/src/main/java/org/openscience/cdk/sgroup/Sgroup.java
@@ -114,14 +114,22 @@ public final Set<Sgroup> getParents() {
     }
 
     /**
-     * Add a bond to this Sgroup.
+     * Add an atom to this Sgroup.
      *
      * @param atom the atom
      */
     public final void addAtom(IAtom atom) {
         this.atoms.add(atom);
     }
 
+    /**
+     * Remove an atom from this Sgroup.
+     * @param atom the atom
+     */
+    public final void removeAtom(IAtom atom) {
+        this.atoms.remove(atom);
+    }
+
     /**
      * Add a bond to this Sgroup. The bond list
      *
@@ -131,6 +139,14 @@ public final void addBond(IBond bond) {
         this.bonds.add(bond);
     }
 
+    /**
+     * Remove a bond from this Sgroup.
+     * @param bond the bond
+     */
+    public final void removeBond(IBond bond) {
+        this.bonds.remove(bond);
+    }
+
     /**
      * Add a parent Sgroup.
      *

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
@@ -30,6 +30,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -735,8 +736,8 @@ public static IAtomContainer suppressHydrogens(IAtomContainer org) {
             return org;
 
         // crossing atoms, positional variation atoms etc
-        Set<IAtom> xatoms = Collections.emptySet();
-        List<Sgroup> sgroups = org.getProperty(CDKConstants.CTAB_SGROUPS);
+        Set<IAtom>         xatoms  = Collections.emptySet();
+        Collection<Sgroup> sgroups = org.getProperty(CDKConstants.CTAB_SGROUPS);
         if (sgroups != null) {
             xatoms = new HashSet<>();
             for (Sgroup sgroup : sgroups) {
@@ -758,7 +759,8 @@ public static IAtomContainer suppressHydrogens(IAtomContainer org) {
         int nCpyAtoms = 0;
         int nCpyBonds = 0;
 
-        final Set<IAtom> hydrogens = new HashSet<IAtom>(nOrgAtoms);
+        final Set<IAtom> hydrogens        = new HashSet<IAtom>(nOrgAtoms);
+        final Set<IBond> bondsToHydrogens = new HashSet<IBond>();
         final IAtom[] cpyAtoms = new IAtom[nOrgAtoms];
 
         // filter the original container atoms for those that can/can't
@@ -904,6 +906,19 @@ public static IAtomContainer suppressHydrogens(IAtomContainer org) {
             }
         }
 
+        if (sgroups != null) {
+            for (Sgroup sgroup : sgroups) {
+                if (sgroup.getValue(SgroupKey.CtabParentAtomList) != null) {
+                    Collection<IAtom> pal = sgroup.getValue(SgroupKey.CtabParentAtomList);
+                    pal.removeAll(hydrogens);
+                }
+                for (IAtom hydrogen : hydrogens)
+                    sgroup.removeAtom(hydrogen);
+                for (IBond bondToHydrogen : bondsToHydrogens)
+                    sgroup.removeBond(bondToHydrogen);
+            }
+        }
+
         return org;
     }
 

diff --git a/...ard/src/test/java/org/openscience/cdk/tools/manipulator/AtomContainerManipulatorTest.java b/...ard/src/test/java/org/openscience/cdk/tools/manipulator/AtomContainerManipulatorTest.java
@@ -23,12 +23,14 @@
 import static org.hamcrest.CoreMatchers.sameInstance;
 import static org.hamcrest.number.IsCloseTo.closeTo;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -59,6 +61,7 @@
 import org.openscience.cdk.io.ISimpleChemObjectReader;
 import org.openscience.cdk.io.MDLV2000Reader;
 import org.openscience.cdk.isomorphism.UniversalIsomorphismTester;
+import org.openscience.cdk.sgroup.Sgroup;
 import org.openscience.cdk.silent.PseudoAtom;
 import org.openscience.cdk.silent.SilentChemObjectBuilder;
 import org.openscience.cdk.smiles.SmilesGenerator;
@@ -1263,19 +1266,31 @@ public void removeHydrogens_molecularH() throws Exception {
     @Test
     public void testSgroupSuppressionSRU() throws Exception {
         assertRemoveH("CCC([H])CC |Sg:n:1,2,3,4:n:ht|",
-                      "CCCCC");
+                      "CCCCC |Sg:n:1,2,3,4:n:ht|");
+    }
+
+    @Test
+    public void testSgroupSuppressionSRUUpdated() throws Exception {
+        SmilesParser smipar = new SmilesParser(SilentChemObjectBuilder.getInstance());
+        IAtomContainer mol = smipar.parseSmiles("CCC([H])CC |Sg:n:1,2,3,4:n:ht|");
+        AtomContainerManipulator.suppressHydrogens(mol);
+        Collection<Sgroup> sgroups = mol.getProperty(CDKConstants.CTAB_SGROUPS);
+        assertNotNull(sgroups);
+        assertThat(sgroups.size(), is(1));
+        Sgroup sgroup = sgroups.iterator().next();
+        assertThat(sgroup.getAtoms().size(), is(3));
     }
 
     @Test
     public void testSgroupSuppressionPositionalVariation() throws Exception {
         assertRemoveH("*[H].C1=CC=CC=C1 |m:0:2.3.4|",
-                      "*[H].C1=CC=CC=C1");
+                      "*[H].C1=CC=CC=C1 |m:0:2.3.4|");
     }
 
     @Test
     public void testSgroupSuppressionSRUCrossingBond() throws Exception {
         assertRemoveH("CCC[H] |Sg:n:2:n:ht|",
-                      "CCC[H]");
+                      "CCC[H] |Sg:n:2:n:ht|");
     }
 
     @Test
@@ -1320,7 +1335,7 @@ static void assertRemoveH(String smiIn, String smiExp) throws Exception {
         SmilesParser smipar = new SmilesParser(SilentChemObjectBuilder.getInstance());
         IAtomContainer m = smipar.parseSmiles(smiIn);
 
-        String smiAct = SmilesGenerator.isomeric().create(AtomContainerManipulator.removeHydrogens(m));
+        String smiAct = new SmilesGenerator().create(AtomContainerManipulator.removeHydrogens(m));
 
         assertThat(smiAct, is(smiExp));
     }