diff --git a/docs/releasehistory.md b/docs/releasehistory.md index ec3d40f93..e060d3344 100644 --- a/docs/releasehistory.md +++ b/docs/releasehistory.md @@ -14,6 +14,8 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w ### Bugfixes +- [Issue #2172](https://github.com/openforcefield/openff-toolkit/issues/2172): `Molecule.to_inchi`, `Molecule.to_inchikey`, and `Molecule.from_inchi` now fall back to the next registered toolkit when a higher-precedence toolkit raises, instead of aborting on the first toolkit's error. For example, InChI generation for molecules with more than 1024 atoms now falls back to RDKit when OpenEye declines to process them. + ### New features ### Improved documentation and warnings diff --git a/openff/toolkit/_tests/test_toolkits.py b/openff/toolkit/_tests/test_toolkits.py index f48f4c68f..593f32541 100644 --- a/openff/toolkit/_tests/test_toolkits.py +++ b/openff/toolkit/_tests/test_toolkits.py @@ -4333,6 +4333,43 @@ def test_call_raise_first_error(self): raise_exception_types=[], ) + @requires_rdkit + def test_inchi_methods_fall_back_through_toolkits(self): + """InChI conversion methods should skip a higher-precedence toolkit that + raises and fall back to the next one (issue #2172). For example, OpenEye + refuses InChI generation for >1024-atom molecules while RDKit supports + it, so ``to_inchi`` must fall back to RDKit rather than propagating the + first toolkit's error.""" + + class _FailingInChIWrapper(RDKitToolkitWrapper): + """A wrapper that advertises the InChI methods but always fails.""" + + def to_inchi(self, *args, **kwargs): + raise ValueError("simulated to_inchi failure") + + def to_inchikey(self, *args, **kwargs): + raise ValueError("simulated to_inchikey failure") + + def from_inchi(self, *args, **kwargs): + raise ValueError("simulated from_inchi failure") + + registry = ToolkitRegistry(toolkit_precedence=[]) + registry.register_toolkit(_FailingInChIWrapper()) + registry.register_toolkit(RDKitToolkitWrapper()) + + molecule = Molecule.from_smiles("CCO") + + # Each call must skip the failing wrapper and succeed via RDKit, instead + # of aborting on the first toolkit's error. + inchi = molecule.to_inchi(toolkit_registry=registry) + assert inchi.startswith("InChI=") + + inchikey = molecule.to_inchikey(toolkit_registry=registry) + assert len(inchikey) > 10 + + roundtrip = Molecule.from_inchi(inchi, toolkit_registry=registry) + assert roundtrip.n_atoms == molecule.n_atoms + @requires_openeye @requires_ambertools diff --git a/openff/toolkit/topology/molecule.py b/openff/toolkit/topology/molecule.py index fee8216df..33e0a6531 100644 --- a/openff/toolkit/topology/molecule.py +++ b/openff/toolkit/topology/molecule.py @@ -1711,6 +1711,7 @@ def from_inchi( _cls=cls, allow_undefined_stereo=allow_undefined_stereo, name=name, + raise_exception_types=[], ) elif isinstance(toolkit_registry, ToolkitWrapper): toolkit = toolkit_registry @@ -1765,7 +1766,12 @@ def to_inchi( """ if isinstance(toolkit_registry, ToolkitRegistry): - inchi = toolkit_registry.call("to_inchi", self, fixed_hydrogens=fixed_hydrogens) + inchi = toolkit_registry.call( + "to_inchi", + self, + fixed_hydrogens=fixed_hydrogens, + raise_exception_types=[], + ) elif isinstance(toolkit_registry, ToolkitWrapper): toolkit = toolkit_registry inchi = toolkit.to_inchi(self, fixed_hydrogens=fixed_hydrogens) # type: ignore[attr-defined] @@ -1814,7 +1820,12 @@ def to_inchikey( """ if isinstance(toolkit_registry, ToolkitRegistry): - inchi_key = toolkit_registry.call("to_inchikey", self, fixed_hydrogens=fixed_hydrogens) + inchi_key = toolkit_registry.call( + "to_inchikey", + self, + fixed_hydrogens=fixed_hydrogens, + raise_exception_types=[], + ) elif isinstance(toolkit_registry, ToolkitWrapper): toolkit = toolkit_registry inchi_key = toolkit.to_inchikey(self, fixed_hydrogens=fixed_hydrogens) # type: ignore[attr-defined]