When an atom is removed from a molecule remove any Sgroups it is invo…

…lved in.
cdk · Feb 7, 2022 · 18a73cc · 18a73cc
1 parent 6ccb216
commit 18a73cc
Show file tree

Hide file tree

Showing 5 changed files with 115 additions and 16 deletions.
diff --git a/base/data/src/main/java/org/openscience/cdk/AtomContainer.java b/base/data/src/main/java/org/openscience/cdk/AtomContainer.java
@@ -1098,11 +1098,29 @@ public void removeAtom(IAtom atom) {
                     --i;
                 }
             }
-            List<IStereoElement> atomElements = new ArrayList<IStereoElement>(3);
-            for (IStereoElement element : stereoElements) {
-                if (element.contains(atom)) atomElements.add(element);
+
+            // consider any stereochemistry involving this atom is now invalid
+            List<IStereoElement<?,?>> stereoToRemove = new ArrayList<>();
+            for (IStereoElement<?,?> element : stereoElements) {
+                if (element.contains(atom)) stereoToRemove.add(element);
             }
-            stereoElements.removeAll(atomElements);
+            stereoElements.removeAll(stereoToRemove);
+
+            // consider any sgroups involving this atom is now invalid
+            List<Sgroup> sgroups = getProperty(CDKConstants.CTAB_SGROUPS);
+            if (sgroups != null) {
+                List<Sgroup> sgrpToRemove = new ArrayList<>();
+                for (Sgroup sgroup : sgroups) {
+                    if (sgroup.getAtoms().contains(atom))
+                        sgrpToRemove.add(sgroup);
+                }
+                if (!sgrpToRemove.isEmpty()) {
+                    sgroups = new ArrayList<>(sgroups); // ensure mutable
+                    sgroups.removeAll(sgrpToRemove);
+                    setProperty(CDKConstants.CTAB_SGROUPS, sgroups);
+                }
+            }
+
             removeAtomOnly(position);
         }
         notifyChanged();

diff --git a/base/data/src/main/java/org/openscience/cdk/AtomContainer2.java b/base/data/src/main/java/org/openscience/cdk/AtomContainer2.java
@@ -1202,11 +1202,27 @@ public void removeAtom(IAtom atom) {
             }
             numLonePairs = newNumLonePairs;
 
-            List<IStereoElement> atomElements = new ArrayList<>();
-            for (IStereoElement element : stereo) {
-                if (element.contains(atom)) atomElements.add(element);
+            // consider any stereochemistry involving this atom is now invalid
+            List<IStereoElement<?,?>> stereoToRemove = new ArrayList<>();
+            for (IStereoElement<?,?> element : stereo) {
+                if (element.contains(atom)) stereoToRemove.add(element);
+            }
+            stereo.removeAll(stereoToRemove);
+
+            // consider any sgroups involving this atom is now invalid
+            List<Sgroup> sgroups = getProperty(CDKConstants.CTAB_SGROUPS);
+            if (sgroups != null) {
+                List<Sgroup> sgrpToRemove = new ArrayList<>();
+                for (Sgroup sgroup : sgroups) {
+                    if (sgroup.getAtoms().contains(atom))
+                        sgrpToRemove.add(sgroup);
+                }
+                if (!sgrpToRemove.isEmpty()) {
+                    sgroups = new ArrayList<>(sgroups); // ensure mutable
+                    sgroups.removeAll(sgrpToRemove);
+                    setProperty(CDKConstants.CTAB_SGROUPS, sgroups);
+                }
             }
-            stereo.removeAll(atomElements);
 
             removeAtomOnly(atomref.getIndex());
         }

diff --git a/base/silent/src/main/java/org/openscience/cdk/silent/AtomContainer.java b/base/silent/src/main/java/org/openscience/cdk/silent/AtomContainer.java
@@ -1066,11 +1066,29 @@ public void removeAtom(IAtom atom) {
                     --i;
                 }
             }
-            List<IStereoElement> atomElements = new ArrayList<IStereoElement>(3);
-            for (IStereoElement element : stereoElements) {
-                if (element.contains(atom)) atomElements.add(element);
+
+            // consider any stereochemistry involving this atom is now invalid
+            List<IStereoElement<?,?>> stereoToRemove = new ArrayList<>();
+            for (IStereoElement<?,?> element : stereoElements) {
+                if (element.contains(atom)) stereoToRemove.add(element);
             }
-            stereoElements.removeAll(atomElements);
+            stereoElements.removeAll(stereoToRemove);
+
+            // consider any sgroups involving this atom is now invalid
+            List<Sgroup> sgroups = getProperty(CDKConstants.CTAB_SGROUPS);
+            if (sgroups != null) {
+                List<Sgroup> sgrpToRemove = new ArrayList<>();
+                for (Sgroup sgroup : sgroups) {
+                    if (sgroup.getAtoms().contains(atom))
+                        sgrpToRemove.add(sgroup);
+                }
+                if (!sgrpToRemove.isEmpty()) {
+                    sgroups = new ArrayList<>(sgroups); // ensure mutable
+                    sgroups.removeAll(sgrpToRemove);
+                    setProperty(CDKConstants.CTAB_SGROUPS, sgroups);
+                }
+            }
+
             removeAtomOnly(position);
         }
     }

diff --git a/base/silent/src/main/java/org/openscience/cdk/silent/AtomContainer2.java b/base/silent/src/main/java/org/openscience/cdk/silent/AtomContainer2.java
@@ -1165,11 +1165,27 @@ public void removeAtom(IAtom atom) {
             }
             numLonePairs = newNumLonePairs;
 
-            List<IStereoElement> atomElements = new ArrayList<>();
-            for (IStereoElement element : stereo) {
-                if (element.contains(atom)) atomElements.add(element);
+            // consider any stereochemistry involving this atom is now invalid
+            List<IStereoElement<?,?>> stereoToRemove = new ArrayList<>();
+            for (IStereoElement<?,?> element : stereo) {
+                if (element.contains(atom)) stereoToRemove.add(element);
+            }
+            stereo.removeAll(stereoToRemove);
+
+            // consider any sgroups involving this atom is now invalid
+            List<Sgroup> sgroups = getProperty(CDKConstants.CTAB_SGROUPS);
+            if (sgroups != null) {
+                List<Sgroup> sgrpToRemove = new ArrayList<>();
+                for (Sgroup sgroup : sgroups) {
+                    if (sgroup.getAtoms().contains(atom))
+                        sgrpToRemove.add(sgroup);
+                }
+                if (!sgrpToRemove.isEmpty()) {
+                    sgroups = new ArrayList<>(sgroups); // ensure mutable
+                    sgroups.removeAll(sgrpToRemove);
+                    setProperty(CDKConstants.CTAB_SGROUPS, sgroups);
+                }
             }
-            stereo.removeAll(atomElements);
 
             removeAtomOnly(atomref.getIndex());
         }

diff --git a/base/test/src/main/java/org/openscience/cdk/test/interfaces/AbstractAtomContainerTest.java b/base/test/src/main/java/org/openscience/cdk/test/interfaces/AbstractAtomContainerTest.java
@@ -3293,4 +3293,35 @@ public void getSelfBond() {
         mol.addBond(b2);
         assertThat(mol.getBond(a1, a1), is(nullValue()));
     }
+
+    @Test public void removeSgroupWithAtom() throws CloneNotSupportedException {
+        IAtomContainer mol = (IAtomContainer) newChemObject();
+        IAtom          a1  = mol.getBuilder().newAtom();
+        IAtom          a2  = mol.getBuilder().newAtom();
+        IAtom          a3  = mol.getBuilder().newAtom();
+        IBond          b1  = mol.getBuilder().newBond();
+        IBond          b2  = mol.getBuilder().newBond();
+        b1.setAtom(a1, 0);
+        b1.setAtom(a2, 1);
+        b2.setAtom(a2, 0);
+        b2.setAtom(a3, 1);
+        mol.addAtom(a1);
+        mol.addAtom(a2);
+        mol.addAtom(a3);
+        mol.addBond(b1);
+        mol.addBond(b2);
+        Sgroup sgroup = new Sgroup();
+        sgroup.setType(SgroupType.CtabStructureRepeatUnit);
+        sgroup.setSubscript("n");
+        sgroup.addAtom(a2);
+        sgroup.addBond(b1);
+        sgroup.addBond(b2);
+        mol.setProperty(CDKConstants.CTAB_SGROUPS,
+                        Collections.singletonList(sgroup));
+        Assert.assertEquals(1,
+                            mol.getProperty(CDKConstants.CTAB_SGROUPS, List.class).size());
+        mol.removeAtom(a2);
+        Assert.assertEquals(0,
+                mol.getProperty(CDKConstants.CTAB_SGROUPS, List.class).size());
+    }
 }