From 6f13a237c6979d73118e81046917cded89509a42 Mon Sep 17 00:00:00 2001 From: Vincent Gao Date: Mon, 15 Jun 2026 17:46:10 +0200 Subject: [PATCH] Fall back through toolkits for InChI conversion methods `Molecule.to_inchi`, `to_inchikey`, and `from_inchi` called `ToolkitRegistry.call` without `raise_exception_types=[]`, so they used the default behavior of re-raising the first toolkit's exception instead of falling back to the next registered toolkit. For example, OpenEye refuses InChI generation for molecules larger than 1024 atoms, but the registry never fell back to RDKit, which supports them. Pass `raise_exception_types=[]` at these call sites, matching the convention already used by the rest of the Molecule API, so a higher-precedence toolkit that raises is skipped in favor of the next one. A single aggregate ValueError is still raised if every toolkit fails. Closes #2172. --- docs/releasehistory.md | 2 ++ openff/toolkit/_tests/test_toolkits.py | 37 ++++++++++++++++++++++++++ openff/toolkit/topology/molecule.py | 15 +++++++++-- 3 files changed, 52 insertions(+), 2 deletions(-) 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]