Skip to content

Commit

Permalink
Raise error when encountering empty InChi string (#1898)
Browse files Browse the repository at this point in the history
* Raise error when encountering empty InChi string

* Fix test regex, lint/merge
  • Loading branch information
mattwthompson authored Jun 21, 2024
1 parent 24b2d7f commit cce000c
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 11 deletions.
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ ci:
files: ^openff|(^examples/((?!deprecated).)*$)|^docs|(^utilities/((?!deprecated).)*$)
repos:
- repo: https://github.com/psf/black
rev: 24.3.0
rev: 24.4.2
hooks:
- id: black
- id: black-jupyter
Expand All @@ -12,7 +12,7 @@ repos:
hooks:
- id: isort
- repo: https://github.com/PyCQA/flake8
rev: 7.0.0
rev: 7.1.0
hooks:
- id: flake8
files: ^openff|(^utilities/((?!deprecated).)*$)
Expand Down
21 changes: 21 additions & 0 deletions openff/toolkit/_tests/test_toolkits.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
ChargeMethodUnavailableError,
ChemicalEnvironmentParsingError,
ConformerGenerationError,
EmptyInChiError,
InChIParseError,
IncorrectNumConformersError,
IncorrectNumConformersWarning,
Expand Down Expand Up @@ -842,6 +843,16 @@ def test_from_bad_inchi(self):
with pytest.raises(InChIParseError, match="ksbfksfksfksbfks"):
Molecule.from_inchi(inchi, toolkit_registry=toolkit)

def test_empty_inchi(self):
"""Reproduce Issue #1897"""
with pytest.raises(
EmptyInChiError,
match="failed to generate",
):
Molecule.from_mapped_smiles("[H:5][S:3]#[N+:2][S:1][H:4]").to_inchi(
toolkit_registry=OpenEyeToolkitWrapper()
)

@pytest.mark.parametrize(
"molecule",
get_mini_drug_bank(
Expand Down Expand Up @@ -2321,6 +2332,16 @@ def test_non_standard_inchi_round_trip(self, molecule):
mol2, bond_order_matching=False, toolkit_registry=toolkit
)

def test_empty_inchi(self):
"""Reproduce Issue #1897"""
with pytest.raises(
EmptyInChiError,
match="failed to generate",
):
Molecule.from_mapped_smiles("[H:5][S:3]#[N+:2][S:1][H:4]").to_inchi(
toolkit_registry=RDKitToolkitWrapper(),
)

def test_smiles_charged(self):
"""Test RDKitWrapper functions for reading/writing charged SMILES"""
toolkit_wrapper = RDKitToolkitWrapper()
Expand Down
8 changes: 8 additions & 0 deletions openff/toolkit/utils/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,14 @@ class SmilesParsingError(OpenFFToolkitException):
"""


class EmptyInChiError(OpenFFToolkitException):
"""
This error is raised when a toolkit returns an empty InChi string, possibly due to bugs
in the underlying InChi code. For more context, see
https://github.com/openforcefield/openff-toolkit/issues/1897
"""


class NotAttachedToMoleculeError(OpenFFToolkitException):
"""Exception for when a component does not belong to a Molecule object, but is queried"""

Expand Down
6 changes: 6 additions & 0 deletions openff/toolkit/utils/openeye_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
ChargeCalculationError,
ChargeMethodUnavailableError,
ConformerGenerationError,
EmptyInChiError,
GAFFAtomTypeWarning,
InChIParseError,
InconsistentStereochemistryError,
Expand Down Expand Up @@ -1916,6 +1917,11 @@ def to_inchi(self, molecule: "Molecule", fixed_hydrogens: bool = False) -> str:
else:
inchi = oechem.OEMolToSTDInChI(oemol)

if len(inchi) == 0:
raise EmptyInChiError(
"OEChem failed to generate an InChI for the molecule."
)

return inchi

def to_inchikey(self, molecule: "Molecule", fixed_hydrogens: bool = False) -> str:
Expand Down
5 changes: 5 additions & 0 deletions openff/toolkit/utils/rdkit_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
AmbiguousBondChemicalAssignment,
ChargeMethodUnavailableError,
ConformerGenerationError,
EmptyInChiError,
InChIParseError,
InvalidAromaticityModelError,
MoleculeParseError,
Expand Down Expand Up @@ -2769,6 +2770,10 @@ def to_inchi(self, molecule: "Molecule", fixed_hydrogens: bool = False):
inchi = Chem.MolToInchi(rdmol, options="-FixedH")
else:
inchi = Chem.MolToInchi(rdmol)

if len(inchi) == 0:
raise EmptyInChiError("RDKit failed to generate an InChI for the molecule.")

return inchi

def to_inchikey(self, molecule: "Molecule", fixed_hydrogens: bool = False) -> str:
Expand Down
26 changes: 17 additions & 9 deletions utilities/test_plugins/custom_plugins/handler_plugins.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
# noqa: INP001
from openff.toolkit.typing.engines.smirnoff import ParameterHandler, ParameterIOHandler
from openff.toolkit.typing.engines.smirnoff.parameters import ParameterAttribute, ParameterType, _NonbondedHandler
from openff.toolkit import unit
from openff.toolkit.typing.engines.smirnoff import ParameterHandler, ParameterIOHandler
from openff.toolkit.typing.engines.smirnoff.parameters import (
ParameterAttribute,
ParameterType,
_NonbondedHandler,
)


class CustomHandler(ParameterHandler):
_TAGNAME = "CustomHandler"
Expand All @@ -14,6 +19,7 @@ class WrongSubclass(list):
class CustomIOHandler(ParameterIOHandler):
_FORMAT = "JSON"


class FOOBuckinghamHandler(_NonbondedHandler):
"""A custom parameter handler for buckingham interactions."""

Expand All @@ -23,18 +29,20 @@ class FOOBuckinghamType(ParameterType):
_ELEMENT_NAME = "Atom"

# Define unit as a Unit object
a = ParameterAttribute(default=None,
unit=unit.kilojoule_per_mole,
)
a = ParameterAttribute(
default=None,
unit=unit.kilojoule_per_mole,
)

# Define using a string
b = ParameterAttribute(default=None,
unit="nanometer**-1",
)
b = ParameterAttribute(
default=None,
unit="nanometer**-1",
)

c = ParameterAttribute(
default=None,
unit=unit.kilojoule_per_mole * unit.nanometer ** 6,
unit=unit.kilojoule_per_mole * unit.nanometer**6,
)

_TAGNAME = "FOOBuckingham"
Expand Down

0 comments on commit cce000c

Please sign in to comment.