Skip to content

Commit 2e9ba69

Browse files
committed
Double-check SSSR to_rdkit_mol for fragment compat
Fragments will sometimes call `get_smallest_set_of_smallest_rings` (e.g. for drawing), which will then call the _fragment_ version of `to_rdkit_mol` (rather than Molecule, since Fragment inherits from Molecule), which returns a _tuple_ rather than a _mol_. This causes a crash. I considerd just replacing this with `converter.to_rdkit_mol` without the checks, but then you'd lose out on any fragment-related benefits from to_rdkit_mol (for example, you need to replace the fragments with H atoms). This commit also adds a check so that the user is at least aware that the default behavior is to change the kwarg to forcibly return mapping=True for fragments.
1 parent 1699791 commit 2e9ba69

File tree

2 files changed

+13
-2
lines changed

2 files changed

+13
-2
lines changed

rmgpy/molecule/fragment.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,12 @@ def to_rdkit_mol(self, *args, **kwargs):
504504

505505
mol0, mapping = self.get_representative_molecule("minimal", update=False)
506506

507-
kwargs["return_mapping"] = True
507+
return_mapping = kwargs.get("return_mapping", False)
508+
if return_mapping == False:
509+
kwargs["return_mapping"] = True
510+
logging.warning("Fragment to_rdkit_mol expects to return a tuple. "
511+
"Setting return_mapping = True; please double-check your code to ensure this is what you want.")
512+
508513
rdmol, rdAtomIdx_mol0 = converter.to_rdkit_mol(
509514
mol0,
510515
*args,

rmgpy/molecule/molecule.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2793,7 +2793,13 @@ def get_smallest_set_of_smallest_rings(self):
27932793

27942794
sssr = []
27952795
# Get the symmetric SSSR using RDKit
2796-
rdkit_mol = self.to_rdkit_mol(remove_h=False, sanitize="partial", save_order=True)
2796+
rdkit_result = self.to_rdkit_mol(remove_h=False, sanitize="partial", save_order=True)
2797+
2798+
if isinstance(rdkit_result, tuple): # can be a tuple if Fragment version of to_rdkit_mol is used
2799+
rdkit_mol = rdkit_result[0]
2800+
else:
2801+
rdkit_mol = rdkit_result
2802+
27972803
ring_info = Chem.GetSymmSSSR(rdkit_mol)
27982804
for ring in ring_info:
27992805
atom_ring = [self.atoms[idx] for idx in ring]

0 commit comments

Comments
 (0)