Fixes #6945 (#6948)

* update default value * suggested fix and a test (#19) * fix mistakes in that last PR (#21) --------- Co-authored-by: Greg Landrum <greg.landrum@gmail.com>
rdkit · Dec 5, 2023 · 1618cf3 · 1618cf3
1 parent a2f3629
commit 1618cf3
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 54 deletions.
diff --git a/Code/GraphMol/Wrap/rough_test.py b/Code/GraphMol/Wrap/rough_test.py
@@ -4001,8 +4001,8 @@ def testResMolSupplier(self):
     self.assertTrue(resMolSuppl.GetIsEnumerated())
     self.assertTrue(
       (resMolSuppl[0].GetBondBetweenAtoms(0, 1).GetBondType() != resMolSuppl[1].GetBondBetweenAtoms(
-        0, 1).GetBondType()) or (resMolSuppl[0].GetBondBetweenAtoms(9, 10).GetBondType() !=
-                                 resMolSuppl[1].GetBondBetweenAtoms(9, 10).GetBondType()))
+        0, 1).GetBondType()) or (resMolSuppl[0].GetBondBetweenAtoms(9, 10).GetBondType()
+                                 != resMolSuppl[1].GetBondBetweenAtoms(9, 10).GetBondType()))
 
     resMolSuppl = Chem.ResonanceMolSupplier(mol, Chem.KEKULE_ALL)
     self.assertEqual(len(resMolSuppl), 8)
@@ -6560,8 +6560,28 @@ def testNewFindMolChiralCenters(self):
     self.assertEqual(len(ctrs), 2)
     self.assertEqual(ctrs, [(1, 'S'), (5, '?')])
 
-  @unittest.skipUnless(hasattr(Chem,'MolFromPNGFile'),
-                     "RDKit not built with iostreams support")
+  def testGithub6945(self):
+    origVal = Chem.GetUseLegacyStereoPerception()
+    tgt = [(1, '?'), (4, 'R')]
+    try:
+      for opt in (not origVal, origVal):
+        Chem.SetUseLegacyStereoPerception(opt)
+        # make sure calling with the default value works:
+        self.assertEqual(
+          tgt,
+          Chem.FindMolChiralCenters(Chem.MolFromSmiles('FC(Cl)(Br)[C@H](F)Cl'),
+                                    includeUnassigned=True))
+        for useLegacy in (True, False):
+          self.assertEqual(
+            tgt,
+            Chem.FindMolChiralCenters(Chem.MolFromSmiles('FC(Cl)(Br)[C@H](F)Cl'),
+                                      includeUnassigned=True, useLegacyImplementation=useLegacy))
+    except:
+      raise
+    finally:
+      Chem.SetUseLegacyStereoPerception(origVal)
+
+  @unittest.skipUnless(hasattr(Chem, 'MolFromPNGFile'), "RDKit not built with iostreams support")
   def testMolFromPNG(self):
     fileN = os.path.join(RDConfig.RDBaseDir, 'Code', 'GraphMol', 'FileParsers', 'test_data',
                          'colchicine.png')
@@ -6575,8 +6595,7 @@ def testMolFromPNG(self):
     self.assertIsNotNone(mol)
     self.assertEqual(mol.GetNumAtoms(), 29)
 
-  @unittest.skipUnless(hasattr(Chem,'MolFromPNGFile'),
-                     "RDKit not built with iostreams support")
+  @unittest.skipUnless(hasattr(Chem, 'MolFromPNGFile'), "RDKit not built with iostreams support")
   def testMolToPNG(self):
     fileN = os.path.join(RDConfig.RDBaseDir, 'Code', 'GraphMol', 'FileParsers', 'test_data',
                          'colchicine.no_metadata.png')
@@ -6597,8 +6616,7 @@ def testMolToPNG(self):
     self.assertIsNotNone(mol)
     self.assertEqual(mol.GetNumAtoms(), 29)
 
-  @unittest.skipUnless(hasattr(Chem,'MolFromPNGFile'),
-                     "RDKit not built with iostreams support")
+  @unittest.skipUnless(hasattr(Chem, 'MolFromPNGFile'), "RDKit not built with iostreams support")
   def testMolsFromPNG(self):
     refMols = [Chem.MolFromSmiles(x) for x in ('c1ccccc1', 'CCO', 'CC(=O)O', 'c1ccccn1')]
     fileN = os.path.join(RDConfig.RDBaseDir, 'Code', 'GraphMol', 'FileParsers', 'test_data',
@@ -6608,8 +6626,7 @@ def testMolsFromPNG(self):
     for mol, refMol in zip(mols, refMols):
       self.assertEqual(Chem.MolToSmiles(mol), Chem.MolToSmiles(refMol))
 
-  @unittest.skipUnless(hasattr(Chem,'MolFromPNGFile'),
-                     "RDKit not built with iostreams support")
+  @unittest.skipUnless(hasattr(Chem, 'MolFromPNGFile'), "RDKit not built with iostreams support")
   def testMetadataToPNG(self):
     fileN = os.path.join(RDConfig.RDBaseDir, 'Code', 'GraphMol', 'FileParsers', 'test_data',
                          'colchicine.png')
@@ -7357,12 +7374,11 @@ def testTranslateChiralFlag(self):
 
   def testHasQueryHs(self):
     for sma, hasQHs in [
-        ("[#1]", (True, False)),
-        ("[#1,N]", (True, True)),
-        ("[$(C-[H])]", (True, False)),
-        ("[$([C,#1])]", (True, True)),
-        ("[$(c([C;!R;!$(C-[N,O,S]);!$(C-[H])](=O))1naaaa1),$(c([C;!R;!$(C-[N,O,S]);!$(C-[H])](=O))1naa[n,s,o]1)]",
-         (True, False))]:
+      ("[#1]", (True, False)), ("[#1,N]", (True, True)), ("[$(C-[H])]", (True, False)),
+      ("[$([C,#1])]", (True, True)),
+      ("[$(c([C;!R;!$(C-[N,O,S]);!$(C-[H])](=O))1naaaa1),$(c([C;!R;!$(C-[N,O,S]);!$(C-[H])](=O))1naa[n,s,o]1)]",
+       (True, False))
+    ]:
       pat = Chem.MolFromSmarts(sma)
       self.assertEqual(Chem.HasQueryHs(pat), hasQHs)
 
@@ -7377,25 +7393,26 @@ def testMolHasQuery(self):
     self.assertTrue(m3.HasQuery())
 
   def testMrvHandling(self):
-    fn1 = os.path.join(RDConfig.RDBaseDir,'Code','GraphMol','MarvinParse','test_data','aspirin.mrv')
+    fn1 = os.path.join(RDConfig.RDBaseDir, 'Code', 'GraphMol', 'MarvinParse', 'test_data',
+                       'aspirin.mrv')
     mol = Chem.MolFromMrvFile(fn1)
     self.assertIsNotNone(mol)
-    self.assertEqual(mol.GetNumAtoms(),13)
+    self.assertEqual(mol.GetNumAtoms(), 13)
     mrv = Chem.MolToMrvBlock(mol)
     self.assertTrue('<molecule molID="m1">' in mrv)
     self.assertFalse('<reaction>' in mrv)
 
     fName = tempfile.NamedTemporaryFile(suffix='.mrv').name
     self.assertFalse(os.path.exists(fName))
-    Chem.MolToMrvFile(mol,fName)
+    Chem.MolToMrvFile(mol, fName)
     self.assertTrue(os.path.exists(fName))
     os.unlink(fName)
 
-    with open(fn1,'r') as inf:
+    with open(fn1, 'r') as inf:
       ind = inf.read()
     mol = Chem.MolFromMrvBlock(ind)
     self.assertIsNotNone(mol)
-    self.assertEqual(mol.GetNumAtoms(),13)
+    self.assertEqual(mol.GetNumAtoms(), 13)
 
 
 if __name__ == '__main__':

diff --git a/rdkit/Chem/__init__.py b/rdkit/Chem/__init__.py
@@ -139,7 +139,7 @@ def SupplierFromFilename(fileN, delim='', **kwargs):
 
 
 def FindMolChiralCenters(mol, force=True, includeUnassigned=False, includeCIP=True,
-                         useLegacyImplementation=True):
+                         useLegacyImplementation=None):
   """
     >>> from rdkit import Chem
     >>> mol = Chem.MolFromSmiles('[C@H](Cl)(F)Br')
@@ -188,40 +188,47 @@ def FindMolChiralCenters(mol, force=True, includeUnassigned=False, includeCIP=Tr
     [(2, 'Tet_CCW'), (4, 'Tet_CCW'), (6, 'Tet_CCW')]
 
   """
-  if useLegacyImplementation:
-    AssignStereochemistry(mol, force=force, flagPossibleStereoCenters=includeUnassigned)
-    centers = []
-    for atom in mol.GetAtoms():
-      if atom.HasProp('_CIPCode'):
-        centers.append((atom.GetIdx(), atom.GetProp('_CIPCode')))
-      elif includeUnassigned and atom.HasProp('_ChiralityPossible'):
-        centers.append((atom.GetIdx(), '?'))
-  else:
-    centers = []
-    itms = FindPotentialStereo(mol)
-    if includeCIP:
-      atomsToLabel = []
-      bondsToLabel = []
+  origUseLegacyVal = GetUseLegacyStereoPerception()
+  if useLegacyImplementation is None:
+    useLegacyImplementation = origUseLegacyVal
+  SetUseLegacyStereoPerception(useLegacyImplementation)
+  try:
+    if useLegacyImplementation:
+      AssignStereochemistry(mol, force=force, flagPossibleStereoCenters=includeUnassigned)
+      centers = []
+      for atom in mol.GetAtoms():
+        if atom.HasProp('_CIPCode'):
+          centers.append((atom.GetIdx(), atom.GetProp('_CIPCode')))
+        elif includeUnassigned and atom.HasProp('_ChiralityPossible'):
+          centers.append((atom.GetIdx(), '?'))
+    else:
+      centers = []
+      itms = FindPotentialStereo(mol)
+      if includeCIP:
+        atomsToLabel = []
+        bondsToLabel = []
+        for si in itms:
+          if si.type == StereoType.Atom_Tetrahedral:
+            atomsToLabel.append(si.centeredOn)
+          elif si.type == StereoType.Bond_Double:
+            bondsToLabel.append(si.centeredOn)
+        AssignCIPLabels(mol, atomsToLabel=atomsToLabel, bondsToLabel=bondsToLabel)
       for si in itms:
-        if si.type == StereoType.Atom_Tetrahedral:
-          atomsToLabel.append(si.centeredOn)
-        elif si.type == StereoType.Bond_Double:
-          bondsToLabel.append(si.centeredOn)
-      AssignCIPLabels(mol, atomsToLabel=atomsToLabel, bondsToLabel=bondsToLabel)
-    for si in itms:
-      if si.type == StereoType.Atom_Tetrahedral and (includeUnassigned
-                                                     or si.specified == StereoSpecified.Specified):
-        idx = si.centeredOn
-        atm = mol.GetAtomWithIdx(idx)
-        if includeCIP and atm.HasProp("_CIPCode"):
-          code = atm.GetProp("_CIPCode")
-        else:
-          if si.specified:
-            code = str(si.descriptor)
+        if si.type == StereoType.Atom_Tetrahedral and (includeUnassigned or si.specified
+                                                       == StereoSpecified.Specified):
+          idx = si.centeredOn
+          atm = mol.GetAtomWithIdx(idx)
+          if includeCIP and atm.HasProp("_CIPCode"):
+            code = atm.GetProp("_CIPCode")
           else:
-            code = '?'
-            atm.SetIntProp('_ChiralityPossible', 1)
-        centers.append((idx, code))
+            if si.specified:
+              code = str(si.descriptor)
+            else:
+              code = '?'
+              atm.SetIntProp('_ChiralityPossible', 1)
+          centers.append((idx, code))
+  finally:
+    SetUseLegacyStereoPerception(origUseLegacyVal)
   return centers