Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Exception rework part 1 #1021

Merged
merged 19 commits into from
Aug 19, 2021
Merged

Exception rework part 1 #1021

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d3c2ac0
Move all exceptions into exceptions.py, make OpenFFToolkitException
mattwthompson Jul 20, 2021
0765c8c
Move all exceptions into exceptions.py
mattwthompson Jul 20, 2021
a3a5494
MessageException -> OpenFFToolkitException
mattwthompson Jul 20, 2021
f845718
Add exceptions to __all__ in parameters.py to satisfy star import
mattwthompson Jul 20, 2021
9ed4edc
Merge remote-tracking branch 'upstream/exception-collection' into exc…
mattwthompson Jul 20, 2021
7075f8a
Cleanup: Absolute imports, do not import from BaseException
mattwthompson Jul 20, 2021
952ec75
Fix duplicate ParseError and when deprecation warnings are raised
mattwthompson Jul 20, 2021
79638a0
Fix passing error messages through exceptions
mattwthompson Jul 20, 2021
df44f3e
Use PEP 562 to control when deprecation warnings are emitted
mattwthompson Jul 20, 2021
91c4e21
Merge remote-tracking branch 'upstream/master' into exception-collection
mattwthompson Aug 10, 2021
71894b9
Apply suggestions from code review
mattwthompson Aug 12, 2021
e7d5edf
Review feedback, cleanup and fixes
mattwthompson Aug 12, 2021
a477843
Update openff/toolkit/utils/exceptions.py
mattwthompson Aug 12, 2021
efe441e
Make OpenFFToolkitException import from _DeprecatedMessageException
mattwthompson Aug 18, 2021
2348feb
Add TODOs for deprecation plan
mattwthompson Aug 18, 2021
a9bc396
Merge remote-tracking branch 'upstream/master' into exception-collection
mattwthompson Aug 19, 2021
cb5dfc0
Make all exceptions import from `OpenFFToolkitException`, note duplicate
mattwthompson Aug 19, 2021
85a00ba
Let `MessageException` be imported from `utils.toolkits` with warning
mattwthompson Aug 19, 2021
c7a2f7c
Merge remote-tracking branch 'upstream/exception-collection' into exc…
mattwthompson Aug 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions docs/releasehistory.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ Releases follow the `major.minor.micro` scheme recommended by [PEP440](https://w

## 0.10.0 Improvements for force field fitting

### Behaviors changed

- [PR #1021](https://github.com/openforcefield/openforcefield/pull/1021): Renames
[`openff.toolkit.utils.exceptions.ParseError`](openff.toolkit.utils.exceptions.ParseError) to
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does adding this link do anything? Clicking around https://github.com/openforcefield/openff-toolkit/blob/master/docs/releasehistory.md just takes me to a bunch of 404 pages. The links on the built docs don't work either -- the only working links come from sphinx rst markup

PR #799 <https://github.com/openforcefield/openff-toolkit/pull/799>: Closes Issue #746 <https://github.com/openforcefield/openff-toolkit/issues/746> by adding :py:meth:Molecule.smirnoff_impropers <openff.toolkit.topology.FrozenMolecule.smirnoff_impropers>, :py:meth:Molecule.amber_impropers <openff.toolkit.topology.FrozenMolecule.amber_impropers>, :py:meth:TopologyMolecule.smirnoff_impropers <openff.toolkit.topology.TopologyMolecule.smirnoff_impropers>, :py:meth:TopologyMolecule.amber_impropers <openff.toolkit.topology.TopologyMolecule.amber_impropers>, :py:meth:Topology.smirnoff_impropers <openff.toolkit.topology.Topology.smirnoff_impropers>, and :py:meth:Topology.amber_impropers <openff.toolkit.topology.Topology.amber_impropers>.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I'm pretty sloppy with release notes between releases, since Jeff and/or Josh will clean them up before making an actual release.

But in this case it doesn't really make sense to try to link to an exception that no longer exists, does it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment was more about the []() linking, where the link isn't a real URL but a qualified path name. Does that work in rendered docs anywhere?

[`openff.toolkit.utils.exceptions.SMILESParseError`](openff.toolkit.utils.exceptions.SMILESParseError) to
avoid a conflict with an identically-named exception in the SMIRNOFF XML parsing code.
- [PR #1021](https://github.com/openforcefield/openforcefield/pull/1021): Renames and moves
[`openff.toolkit.typing.engines.smirnoff.forcefield.ParseError`](openff.toolkit.typing.engines.smirnoff.forcefield.ParseError) to
[`openff.toolkit.utils.exceptions.SMIRNOFFParseError`](openff.toolkit.utils.exceptions.SMIRNOFFParseError).
This `ParseError` is deprecated and will be removed in a future release.

## Current Development

### New features and behaviors changed

- [PR #1027](https://github.com/openforcefield/openforcefield/pull/1027): Corrects interconversion of Molecule objects
Expand Down
5 changes: 0 additions & 5 deletions openff/toolkit/tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@

"""


# =============================================================================================
# GLOBAL IMPORTS
# =============================================================================================

import logging

import pytest
Expand Down
5 changes: 0 additions & 5 deletions openff/toolkit/tests/create_molecules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@

"""

# =============================================================================================
# GLOBAL IMPORTS
# =============================================================================================


import numpy as np
from simtk import unit

Expand Down
4 changes: 0 additions & 4 deletions openff/toolkit/tests/test_examples.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@
Test that the examples in the repo run without errors.
"""

# ======================================================================
# GLOBAL IMPORTS
# ======================================================================

import os
import pathlib
import re
Expand Down
30 changes: 11 additions & 19 deletions openff/toolkit/tests/test_forcefield.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@
"""


# =============================================================================================
# GLOBAL IMPORTS
# =============================================================================================

import copy
import itertools
import os
Expand Down Expand Up @@ -49,21 +45,25 @@
from openff.toolkit.typing.engines.smirnoff import (
ElectrostaticsHandler,
ForceField,
FractionalBondOrderInterpolationMethodUnsupportedError,
IncompatibleParameterError,
LibraryChargeHandler,
ParameterHandler,
ParameterLookupError,
PartialChargeVirtualSitesError,
SMIRNOFFAromaticityError,
SMIRNOFFSpecError,
SMIRNOFFSpecUnimplementedError,
ToolkitAM1BCCHandler,
XMLParameterIOHandler,
get_available_force_fields,
vdWHandler,
)
from openff.toolkit.utils import get_data_file_path
from openff.toolkit.utils.exceptions import (
FractionalBondOrderInterpolationMethodUnsupportedError,
IncompatibleParameterError,
NonintegralMoleculeChargeException,
ParameterLookupError,
PartialChargeVirtualSitesError,
SMIRNOFFAromaticityError,
SMIRNOFFSpecError,
SMIRNOFFSpecUnimplementedError,
UnassignedMoleculeChargeException,
)
from openff.toolkit.utils.toolkits import (
AmberToolsToolkitWrapper,
ChargeMethodUnavailableError,
Expand Down Expand Up @@ -2322,10 +2322,6 @@ def test_nonintegral_charge_exception(self, toolkit_registry, registry_descripti
"""Test skipping charge generation and instead getting charges from the original Molecule"""
from simtk.openmm import app

from openff.toolkit.typing.engines.smirnoff.parameters import (
NonintegralMoleculeChargeException,
)

# Create an ethanol molecule without using a toolkit
ethanol = create_ethanol()
ethanol.partial_charges[0] = 1.0 * unit.elementary_charge
Expand Down Expand Up @@ -3176,10 +3172,6 @@ def test_library_charges_dont_parameterize_molecule_because_of_incomplete_covera
"""Fail to assign charges to a molecule because not all atoms can be assigned"""
from simtk.openmm import NonbondedForce

from openff.toolkit.typing.engines.smirnoff.parameters import (
UnassignedMoleculeChargeException,
)

molecules = [Molecule.from_file(get_data_file_path("molecules/toluene.sdf"))]
top = Topology.from_molecules(molecules)

Expand Down
5 changes: 0 additions & 5 deletions openff/toolkit/tests/test_io.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@

"""


# =============================================================================================
# GLOBAL IMPORTS
# =============================================================================================

import pytest

from openff.toolkit.typing.engines.smirnoff.io import XMLParameterIOHandler
Expand Down
4 changes: 0 additions & 4 deletions openff/toolkit/tests/test_molecule.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@

"""

# =============================================================================================
# GLOBAL IMPORTS
# =============================================================================================

import copy
import os
import pickle
Expand Down
22 changes: 10 additions & 12 deletions openff/toolkit/tests/test_parameters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,42 +9,40 @@

"""


# ======================================================================
# GLOBAL IMPORTS
# ======================================================================

import numpy
import pytest
from numpy.testing import assert_almost_equal
from simtk import unit

from openff.toolkit.topology import Molecule
from openff.toolkit.typing.engines.smirnoff import SMIRNOFFVersionError
from openff.toolkit.typing.engines.smirnoff.parameters import (
BondHandler,
ChargeIncrementModelHandler,
DuplicateParameterError,
GBSAHandler,
ImproperTorsionHandler,
IncompatibleParameterError,
IndexedParameterAttribute,
LibraryChargeHandler,
NotEnoughPointsForInterpolationError,
ParameterAttribute,
ParameterHandler,
ParameterList,
ParameterLookupError,
ParameterType,
ProperTorsionHandler,
SMIRNOFFSpecError,
VirtualSiteHandler,
_linear_inter_or_extrapolate,
_ParameterAttributeHandler,
vdWHandler,
)
from openff.toolkit.utils import IncompatibleUnitError, detach_units
from openff.toolkit.utils import detach_units
from openff.toolkit.utils.collections import ValidatedList
from openff.toolkit.utils.exceptions import (
DuplicateParameterError,
IncompatibleParameterError,
IncompatibleUnitError,
NotEnoughPointsForInterpolationError,
ParameterLookupError,
SMIRNOFFSpecError,
SMIRNOFFVersionError,
)

# ======================================================================
# Test ParameterAttribute descriptor
Expand Down
20 changes: 9 additions & 11 deletions openff/toolkit/tests/test_toolkit_io.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@

"""

# =============================================================================================
# GLOBAL IMPORTS
# =============================================================================================

import os
import pathlib
import sys
Expand All @@ -26,7 +22,11 @@
from openff.toolkit.tests import create_molecules
from openff.toolkit.tests.utils import requires_openeye, requires_rdkit
from openff.toolkit.topology.molecule import Molecule
from openff.toolkit.utils import OpenEyeToolkitWrapper, RDKitToolkitWrapper, exceptions
from openff.toolkit.utils import OpenEyeToolkitWrapper, RDKitToolkitWrapper
from openff.toolkit.utils.exceptions import (
lilyminium marked this conversation as resolved.
Show resolved Hide resolved
SMILESParseError,
UndefinedStereochemistryError,
)

# ================================================================
# Data records used for testing.
Expand Down Expand Up @@ -918,14 +918,14 @@ def _test_3D_sdf_keeps_hydrogens(self, mol):

def test_from_file_with_undefined_stereo(self):
with pytest.raises(
exceptions.UndefinedStereochemistryError,
UndefinedStereochemistryError,
match=f"Unable to make OFFMol from {self.tk_mol_name}: {self.tk_mol_name} has unspecified stereochemistry",
):
self.toolkit_wrapper.from_file(file_manager.chebi_1148_sdf, "sdf")

def test_from_file_obj_with_undefined_stereo(self):
with pytest.raises(
exceptions.UndefinedStereochemistryError,
UndefinedStereochemistryError,
match=f"Unable to make OFFMol from {self.tk_mol_name}: {self.tk_mol_name} has unspecified stereochemistry",
):
self.toolkit_wrapper.from_file_obj(file_obj_manager.chebi_1148_sdf, "sdf")
Expand Down Expand Up @@ -1155,9 +1155,7 @@ def test_parse_methane_with_explicit_Hs_and_say_they_are_explicit(self):
assert mol.n_bonds == 4

def test_parse_bad_smiles(self):
with pytest.raises(
exceptions.SMILESParseError, match="Unable to parse the SMILES string"
):
with pytest.raises(SMILESParseError, match="Unable to parse the SMILES string"):
mol = self.toolkit_wrapper.from_smiles("QWERT")

### Copied from test_toolkits.py
Expand All @@ -1173,7 +1171,7 @@ def test_parse_bad_smiles(self):
)
def test_smiles_missing_stereochemistry(self, title, smiles):
if "unspec" in title:
with pytest.raises(exceptions.UndefinedStereochemistryError):
with pytest.raises(UndefinedStereochemistryError):
self.toolkit_wrapper.from_smiles(smiles)
mol = self.toolkit_wrapper.from_smiles(smiles, allow_undefined_stereo=True)
else:
Expand Down
19 changes: 9 additions & 10 deletions openff/toolkit/tests/test_toolkits.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@

"""

# =============================================================================================
# GLOBAL IMPORTS
# =============================================================================================
import logging
import os
from tempfile import NamedTemporaryFile
Expand All @@ -38,22 +35,24 @@
)
from openff.toolkit.topology.molecule import Molecule
from openff.toolkit.utils import get_data_file_path
from openff.toolkit.utils.toolkits import (
GLOBAL_TOOLKIT_REGISTRY,
AmberToolsToolkitWrapper,
BuiltInToolkitWrapper,
from openff.toolkit.utils.exceptions import (
ChargeMethodUnavailableError,
GAFFAtomTypeWarning,
IncorrectNumConformersError,
IncorrectNumConformersWarning,
InvalidIUPACNameError,
InvalidToolkitError,
ToolkitUnavailableException,
UndefinedStereochemistryError,
)
from openff.toolkit.utils.toolkits import (
GLOBAL_TOOLKIT_REGISTRY,
AmberToolsToolkitWrapper,
BuiltInToolkitWrapper,
GAFFAtomTypeWarning,
OpenEyeToolkitWrapper,
RDKitToolkitWrapper,
ToolkitRegistry,
ToolkitUnavailableException,
ToolkitWrapper,
UndefinedStereochemistryError,
)

# =============================================================================================
Expand Down
15 changes: 6 additions & 9 deletions openff/toolkit/tests/test_topology.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@

"""

# =============================================================================================
# GLOBAL IMPORTS
# =============================================================================================

from unittest import TestCase

import numpy as np
Expand All @@ -32,11 +28,7 @@
requires_rdkit,
)
from openff.toolkit.topology import (
DuplicateUniqueMoleculeError,
ImproperDict,
InvalidBoxVectorsError,
InvalidPeriodicityError,
MissingUniqueMoleculesError,
Molecule,
TagSortedDict,
Topology,
Expand All @@ -49,6 +41,12 @@
OpenEyeToolkitWrapper,
RDKitToolkitWrapper,
)
from openff.toolkit.utils.exceptions import (
DuplicateUniqueMoleculeError,
InvalidBoxVectorsError,
InvalidPeriodicityError,
MissingUniqueMoleculesError,
)

# =============================================================================================
# UTILITY FUNCTIONS
Expand Down Expand Up @@ -1169,7 +1167,6 @@ def test_chemical_environments_matches_RDK(self):
],
)
def test_nth_degree_neighbors(n_degrees, num_pairs):
pass
smiles = ["c1ccccc1", "N1ONON1"]
topology = Topology.from_molecules([Molecule.from_smiles(smi) for smi in smiles])

Expand Down
17 changes: 13 additions & 4 deletions openff/toolkit/tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@

"""

# =============================================================================================
# GLOBAL IMPORTS
# =============================================================================================

import ast
import os

Expand Down Expand Up @@ -133,3 +129,16 @@ def test_sort_smirnoff_dict():
assert smirnoff_dict == OrderedDict(
sort_smirnoff_dict(forcefield._to_smirnoff_data())
)


def test_import_message_exception_raises_warning(caplog):
# TODO: Remove when removing MessageException
msg = "DEPRECATED and will be removed in a future release of the OpenFF Toolkit"

with pytest.warns(DeprecationWarning, match=msg):
from openff.toolkit.utils.exceptions import MessageException

with pytest.warns(None) as rec:
from openff.toolkit.utils.exceptions import SMILESParseError

assert len(rec) == 0
11 changes: 2 additions & 9 deletions openff/toolkit/tests/test_utils_callback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,10 @@

"""

# =====================================================================
# GLOBAL IMPORTS
# =====================================================================

import pytest

from openff.toolkit.utils.callback import (
Callbackable,
CallbackRegistrationError,
callback_method,
)
from openff.toolkit.utils.callback import Callbackable, callback_method
from openff.toolkit.utils.exceptions import CallbackRegistrationError

# =====================================================================
# UTILITY CLASSES AND FUNCTIONS
Expand Down
Loading