Fixes rdkit#6757 (rdkit#7664)

* assignStereochemistry() should call updatePropertyCache() if it is needed * Fixes rdkit#6757 * update expectations in current tests * add another test
e-kwsm · Jul 25, 2024 · 8e57151 · 8e57151
1 parent 3f3ff38
commit 8e57151
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 22 deletions.
diff --git a/Code/GraphMol/Chirality.cpp b/Code/GraphMol/Chirality.cpp
@@ -2771,6 +2771,10 @@ void assignStereochemistry(ROMol &mol, bool cleanIt, bool force,
     return;
   }
 
+  if (mol.needsUpdatePropertyCache()) {
+    mol.updatePropertyCache(false);
+  }
+
   if (!Chirality::getUseLegacyStereoPerception()) {
     Chirality::stereoPerception(mol, cleanIt, flagPossibleStereoCenters);
   } else {

diff --git a/Code/GraphMol/Descriptors/Lipinski.cpp b/Code/GraphMol/Descriptors/Lipinski.cpp
@@ -506,36 +506,46 @@ bool hasStereoAssigned(const ROMol &mol) {
   return mol.hasProp(common_properties::_StereochemDone);
 }
 }  // namespace
-const std::string NumAtomStereoCentersVersion = "1.0.0";
+const std::string NumAtomStereoCentersVersion = "1.0.1";
 unsigned int numAtomStereoCenters(const ROMol &mol) {
+  std::unique_ptr<ROMol> tmol;
+  const ROMol *mptr = &mol;
   if (!hasStereoAssigned(mol)) {
-    throw ValueErrorException(
-        "numStereoCenters called without stereo being assigned");
+    tmol.reset(new ROMol(mol));
+    constexpr bool cleanIt = true;
+    constexpr bool force = true;
+    constexpr bool flagPossible = true;
+    MolOps::assignStereochemistry(*tmol, cleanIt, force, flagPossible);
+    mptr = tmol.get();
   }
 
   unsigned int res = 0;
-  for (ROMol::ConstAtomIterator atom = mol.beginAtoms(); atom != mol.endAtoms();
-       ++atom) {
-    if ((*atom)->hasProp(common_properties::_ChiralityPossible)) {
-      res++;
+  for (const auto &atom : mptr->atoms()) {
+    if (atom->hasProp(common_properties::_ChiralityPossible)) {
+      ++res;
     }
   }
   return res;
 }
 
-const std::string NumUnspecifiedAtomStereoCentersVersion = "1.0.0";
+const std::string NumUnspecifiedAtomStereoCentersVersion = "1.0.1";
 unsigned int numUnspecifiedAtomStereoCenters(const ROMol &mol) {
+  std::unique_ptr<ROMol> tmol;
+  const ROMol *mptr = &mol;
   if (!hasStereoAssigned(mol)) {
-    throw ValueErrorException(
-        "numUnspecifiedStereoCenters called without stereo being assigned");
+    tmol.reset(new ROMol(mol));
+    constexpr bool cleanIt = true;
+    constexpr bool force = true;
+    constexpr bool flagPossible = true;
+    MolOps::assignStereochemistry(*tmol, cleanIt, force, flagPossible);
+    mptr = tmol.get();
   }
 
   unsigned int res = 0;
-  for (ROMol::ConstAtomIterator atom = mol.beginAtoms(); atom != mol.endAtoms();
-       ++atom) {
-    if ((*atom)->hasProp(common_properties::_ChiralityPossible) &&
-        (*atom)->getChiralTag() == Atom::CHI_UNSPECIFIED) {
-      res++;
+  for (const auto &atom : mptr->atoms()) {
+    if (atom->hasProp(common_properties::_ChiralityPossible) &&
+        atom->getChiralTag() == Atom::CHI_UNSPECIFIED) {
+      ++res;
     }
   }
   return res;

diff --git a/Code/GraphMol/Descriptors/catch_tests.cpp b/Code/GraphMol/Descriptors/catch_tests.cpp
@@ -618,4 +618,22 @@ TEST_CASE("Github #7364: BCUT descriptors failing for moleucles with Hs") {
     CHECK(ref.size() == val.size());
     CHECK(ref == val);
   }
+}
+
+TEST_CASE(
+    "Github #6757: numAtomStereoCenters fails if molecule is sanitized a second time") {
+  SECTION("as reported") {
+    auto m = "C[C@H](F)Cl"_smiles;
+    REQUIRE(m);
+    CHECK(Descriptors::numAtomStereoCenters(*m) == 1);
+    CHECK(Descriptors::numUnspecifiedAtomStereoCenters(*m) == 0);
+    MolOps::sanitizeMol(*m);
+    CHECK(Descriptors::numAtomStereoCenters(*m) == 1);
+  }
+  SECTION("expanded") {
+    auto m = "C[C@H](F)C(O)Cl"_smiles;
+    REQUIRE(m);
+    CHECK(Descriptors::numAtomStereoCenters(*m) == 2);
+    CHECK(Descriptors::numUnspecifiedAtomStereoCenters(*m) == 1);
+  }
 }
diff --git a/Code/GraphMol/Descriptors/test.cpp b/Code/GraphMol/Descriptors/test.cpp
@@ -1993,16 +1993,14 @@ void testStereoCounting() {
   names.emplace_back("NumUnspecifiedAtomStereoCenters");
   Properties prop(names);
 
-  try {
-    prop.computeProperties(*m);
-    TEST_ASSERT(0);  // didn't catch exception
-  } catch (ValueErrorException &) {
-    BOOST_LOG(rdErrorLog) << "---Caught stereo value error---" << std::endl;
-  }
+  // the stereo perception is done automatically:
+  std::vector<double> res = prop.computeProperties(*m);
+  TEST_ASSERT(res[0] == 1);
+  TEST_ASSERT(res[1] == 1);
 
   delete m;
   m = SmilesToMol("NC(C)(F)C(=O)O", sanitize);
-  std::vector<double> res = prop.computeProperties(*m);
+  res = prop.computeProperties(*m);
   TEST_ASSERT(res[0] == 1);
   TEST_ASSERT(res[1] == 1);
   delete m;