From 864846fbe3ab08534c61ddb0b59338caf6ab4dc3 Mon Sep 17 00:00:00 2001 From: SimonBoothroyd Date: Sun, 14 Jun 2020 16:52:09 -0600 Subject: [PATCH 01/11] Adds mechanism to register parameter handlers via entrypoints. --- openforcefield/tests/plugins.py | 10 ++ .../tests/test_parameter_plugins.py | 97 +++++++++++++++++++ .../typing/engines/smirnoff/forcefield.py | 11 ++- .../typing/engines/smirnoff/plugins.py | 91 +++++++++++++++++ 4 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 openforcefield/tests/plugins.py create mode 100644 openforcefield/tests/test_parameter_plugins.py create mode 100644 openforcefield/typing/engines/smirnoff/plugins.py diff --git a/openforcefield/tests/plugins.py b/openforcefield/tests/plugins.py new file mode 100644 index 000000000..29fa4730e --- /dev/null +++ b/openforcefield/tests/plugins.py @@ -0,0 +1,10 @@ +"""Contains a set of 'plugin' classes to enable testing of the plugin system.""" +from openforcefield.typing.engines.smirnoff import ParameterHandler, ParameterIOHandler + + +class CustomHandler(ParameterHandler): + _TAGNAME = 'CustomHandler' + + +class CustomIOHandler(ParameterIOHandler): + _FORMAT = 'JSON' diff --git a/openforcefield/tests/test_parameter_plugins.py b/openforcefield/tests/test_parameter_plugins.py new file mode 100644 index 000000000..0a5229107 --- /dev/null +++ b/openforcefield/tests/test_parameter_plugins.py @@ -0,0 +1,97 @@ +""" +Test classes and function in module openforcefield.typing.engines.smirnoff.parameters.plugins +""" +import pkg_resources +import pytest + +from openforcefield.typing.engines.smirnoff import ForceField +from openforcefield.typing.engines.smirnoff.plugins import load_handler_plugins + + +@pytest.yield_fixture() +def mock_entry_point_plugins(): + """Registers a fake parameter handler and io handler with the + entry point plugin system. + + Notes + ----- + This function is based on `this stack overflow answer + `_ + """ + + previous_entries = pkg_resources.working_set.entries + previous_entry_keys = pkg_resources.working_set.entry_keys + previous_by_key = pkg_resources.working_set.by_key + + # Create a fake distribution to insert into the global working_set + distribution = pkg_resources.Distribution(__file__) + + # Create the fake entry point definitions. These include a parameter handler + # which is supported, and an io parameter handler which should be skipped. + handler_entry_point = pkg_resources.EntryPoint.parse( + "CustomHandler = openforcefield.tests.plugins:CustomHandler", + dist=distribution + ) + io_handler_entry_point = pkg_resources.EntryPoint.parse( + "CustomIOHandler = openforcefield.tests.plugins:CustomIOHandler", + dist=distribution + ) + + # Add the mapping to the fake EntryPoint + distribution._ep_map = { + "openff.toolkit.plugins.handlers": { + "CustomHandler": handler_entry_point, + "CustomIOHandler": io_handler_entry_point + } + } + + # Add the fake distribution to the global working_set + pkg_resources.working_set.add(distribution, "CustomHandler") + pkg_resources.working_set.add(distribution, "CustomIOHandler") + + yield + + pkg_resources.working_set.entries = previous_entries + pkg_resources.working_set.entry_keys = previous_entry_keys + pkg_resources.working_set.by_key = previous_by_key + + +def test_load_handler_plugins(mock_entry_point_plugins): + """Tests that parameter handlers can be registered as plugins. + """ + + registered_plugins = load_handler_plugins() + + assert len(registered_plugins) == 1 + assert registered_plugins[0].__name__ == "CustomHandler" + + +def test_force_field_custom_handler(mock_entry_point_plugins): + """Tests a force field can make use of a custom parameter handler registered + through the entrypoint plugin system. + """ + + # Construct a simple FF which only uses the custom handler. + force_field_contents = "\n".join( + [ + "", + "", + " ", + "" + ] + ) + + # An exception should be raised when plugins aren't allowed. + with pytest.raises(KeyError) as error_info: + ForceField(force_field_contents, load_plugins=False) + + assert ( + "Cannot find a registered parameter handler class for tag 'CustomHandler'" in error_info.value.args[0] + ) + + # Otherwise the FF should be created as expected. + force_field = ForceField(force_field_contents) + + parameter_handler = force_field.get_parameter_handler("CustomHandler") + assert parameter_handler is not None + assert parameter_handler.__class__.__name__ == "CustomHandler" diff --git a/openforcefield/typing/engines/smirnoff/forcefield.py b/openforcefield/typing/engines/smirnoff/forcefield.py index 069f9aedb..fbbd44932 100644 --- a/openforcefield/typing/engines/smirnoff/forcefield.py +++ b/openforcefield/typing/engines/smirnoff/forcefield.py @@ -44,6 +44,7 @@ convert_0_1_smirnoff_to_0_2, convert_0_2_smirnoff_to_0_3 from openforcefield.topology.molecule import DEFAULT_AROMATICITY_MODEL from openforcefield.typing.engines.smirnoff.parameters import ParameterHandler +from openforcefield.typing.engines.smirnoff.plugins import load_handler_plugins from openforcefield.typing.engines.smirnoff.io import ParameterIOHandler @@ -221,7 +222,8 @@ def __init__(self, parameter_handler_classes=None, parameter_io_handler_classes=None, disable_version_check=False, - allow_cosmetic_attributes=False): + allow_cosmetic_attributes=False, + load_plugins=True): """Create a new :class:`ForceField` object from one or more SMIRNOFF parameter definition files. Parameters @@ -245,6 +247,9 @@ def __init__(self, This option is primarily intended for forcefield development. allow_cosmetic_attributes : bool, optional. Default = False Whether to retain non-spec kwargs from data sources. + load_plugins: bool, optional. Default = True + Whether to load ``ParameterHandler`` classes which have been registered + by installed plugins. Examples -------- @@ -276,11 +281,15 @@ def __init__(self, # since both will try to register themselves for the same XML tag and an Exception will be raised. if parameter_handler_classes is None: parameter_handler_classes = all_subclasses(ParameterHandler) + if load_plugins: + parameter_handler_classes += load_handler_plugins() + self._register_parameter_handler_classes(parameter_handler_classes) # Register all ParameterIOHandler objects that will process serialized parameter representations if parameter_io_handler_classes is None: parameter_io_handler_classes = all_subclasses(ParameterIOHandler) + self._register_parameter_io_handler_classes( parameter_io_handler_classes) diff --git a/openforcefield/typing/engines/smirnoff/plugins.py b/openforcefield/typing/engines/smirnoff/plugins.py new file mode 100644 index 000000000..3fdb7cd2f --- /dev/null +++ b/openforcefield/typing/engines/smirnoff/plugins.py @@ -0,0 +1,91 @@ +"""This module defines functions for loading parameter handler and parser clases which +have been registered through the `entrypoint plugin system `_. + +.. warning :: + + This feature is experimental and may be removed / altered in future versions. + +Currently only ``ParameterHandler`` classes can be registered via the plugin +system. This is possible by registering the handler class through an entry +point in your projects ``setup.py`` file:: + + setup( + ... + entry_points={ + 'openff.toolkit.plugins.handlers': ['CustomHandler = myapp:CustomHandler'] + }, + ... + ) + +where in this example your package is named ``myapp`` and contains a class which +inherits from ``ParameterHandler`` named ``CustomHandler``. +""" +import logging + +from openforcefield.typing.engines.smirnoff.parameters import ParameterHandler + +logger = logging.getLogger(__name__) + +SUPPORTED_PLUGIN_NAMES = ["handlers"] # io_handlers could be supported in future. + + +def _load_handler_plugins(handler_name, expected_type): + """Loads parameter handler plugins of a specified type which have been registered + with the ``entrypoint`` plugin system. + + Parameters + ---------- + handler_name: str + The name of the hander plugin. This can currently be any of the names + listed in ``SUPPORTED_PLUGIN_NAMES``. + expected_type: type + The expected class type of the plugin. E.g. when loading parameter io + handler plugins the expected class type is ``ParameterIOHandler``. Any + classes not matching the expected type will be skipped. + """ + import pkg_resources + + discovered_plugins = [] + + if handler_name not in SUPPORTED_PLUGIN_NAMES: + raise NotImplementedError() + + for entry_point in pkg_resources.iter_entry_points( + f"openff.toolkit.plugins.{handler_name}" + ): + + try: + discovered_plugins.append(entry_point.load()) + except ImportError: + logger.exception(f"Could not load the {entry_point} plugin") + continue + + valid_plugins = [] + + for discovered_plugin in discovered_plugins: + + if not issubclass(discovered_plugin, expected_type): + + logger.info( + f"The {discovered_plugin.__name__} object has been registered as a " + f"{handler_name} plugin, but does not inherit from " + f"{expected_type.__name__}. This plugin will be skipped." + ) + continue + + valid_plugins.append(discovered_plugin) + + return valid_plugins + + +def load_handler_plugins(): + """Loads any ``ParameterHandler`` class plugins which have been registered through + the ``entrypoint`` plugin system. + + Returns + ------- + list of type of ParameterHandler + The registered ``ParameterHandler`` plugins. + """ + return _load_handler_plugins("handlers", ParameterHandler) From 56c4dc5c9dde63e304073d92f96fe1e787d42485 Mon Sep 17 00:00:00 2001 From: SimonBoothroyd Date: Sun, 14 Jun 2020 16:58:27 -0600 Subject: [PATCH 02/11] Update change log. --- docs/releasehistory.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/releasehistory.rst b/docs/releasehistory.rst index 90433ca5f..b10d4fff9 100644 --- a/docs/releasehistory.rst +++ b/docs/releasehistory.rst @@ -162,7 +162,7 @@ New features - `PR #573 `_: Adds ``quacpac`` error output to ``quacpac`` failure in ``Molecule.compute_partial_charges_am1bcc``. - `PR #560 `_: Added visualization method to the the Molecule class. - +- `PR #620 `_: Added the ability to register parameter handlers via entry point plugins. Behavior changed """""""""""""""" From f963bb2da030c7ad8267b8a2ced56d649620b2aa Mon Sep 17 00:00:00 2001 From: SimonBoothroyd Date: Sun, 14 Jun 2020 17:23:48 -0600 Subject: [PATCH 03/11] Reverse test order. --- .../tests/test_parameter_plugins.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/openforcefield/tests/test_parameter_plugins.py b/openforcefield/tests/test_parameter_plugins.py index 0a5229107..452d378ff 100644 --- a/openforcefield/tests/test_parameter_plugins.py +++ b/openforcefield/tests/test_parameter_plugins.py @@ -56,16 +56,6 @@ def mock_entry_point_plugins(): pkg_resources.working_set.by_key = previous_by_key -def test_load_handler_plugins(mock_entry_point_plugins): - """Tests that parameter handlers can be registered as plugins. - """ - - registered_plugins = load_handler_plugins() - - assert len(registered_plugins) == 1 - assert registered_plugins[0].__name__ == "CustomHandler" - - def test_force_field_custom_handler(mock_entry_point_plugins): """Tests a force field can make use of a custom parameter handler registered through the entrypoint plugin system. @@ -95,3 +85,13 @@ def test_force_field_custom_handler(mock_entry_point_plugins): parameter_handler = force_field.get_parameter_handler("CustomHandler") assert parameter_handler is not None assert parameter_handler.__class__.__name__ == "CustomHandler" + + +def test_load_handler_plugins(mock_entry_point_plugins): + """Tests that parameter handlers can be registered as plugins. + """ + + registered_plugins = load_handler_plugins() + + assert len(registered_plugins) == 1 + assert registered_plugins[0].__name__ == "CustomHandler" From 42752285c4a27b008212cb769a6b45d802eabcd1 Mon Sep 17 00:00:00 2001 From: SimonBoothroyd Date: Sun, 14 Jun 2020 19:26:57 -0600 Subject: [PATCH 04/11] Don't re-register loaded handlers. --- openforcefield/typing/engines/smirnoff/forcefield.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/openforcefield/typing/engines/smirnoff/forcefield.py b/openforcefield/typing/engines/smirnoff/forcefield.py index fbbd44932..fe7a72538 100644 --- a/openforcefield/typing/engines/smirnoff/forcefield.py +++ b/openforcefield/typing/engines/smirnoff/forcefield.py @@ -282,7 +282,15 @@ def __init__(self, if parameter_handler_classes is None: parameter_handler_classes = all_subclasses(ParameterHandler) if load_plugins: - parameter_handler_classes += load_handler_plugins() + + registered_handlers = load_handler_plugins() + + # Make sure the same handlers aren't added twice. + parameter_handler_classes += [ + handler + for handler in registered_handlers + if handler not in parameter_handler_classes + ] self._register_parameter_handler_classes(parameter_handler_classes) From 8f2021895e5a46d460c29cfc0b69a11fc80437b1 Mon Sep 17 00:00:00 2001 From: Jeff Wagner Date: Fri, 19 Jun 2020 15:42:05 -0700 Subject: [PATCH 05/11] Update docs/releasehistory.rst --- docs/releasehistory.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/releasehistory.rst b/docs/releasehistory.rst index c09b641e1..823f4a329 100644 --- a/docs/releasehistory.rst +++ b/docs/releasehistory.rst @@ -162,7 +162,7 @@ New features - `PR #573 `_: Adds ``quacpac`` error output to ``quacpac`` failure in ``Molecule.compute_partial_charges_am1bcc``. - `PR #560 `_: Added visualization method to the the Molecule class. -- `PR #620 `_: Added the ability to register parameter handlers via entry point plugins. +- `PR #620 `_: Added the ability to register parameter handlers via entry point plugins. This functionality is accessible by initializing a ``ForceField`` with the ``load_plugins=True`` keyword argument. Behavior changed """""""""""""""" From 4111809367a9264e853652f1584e6b151580aeb0 Mon Sep 17 00:00:00 2001 From: Jeff Wagner Date: Fri, 19 Jun 2020 15:42:14 -0700 Subject: [PATCH 06/11] Update openforcefield/typing/engines/smirnoff/forcefield.py --- openforcefield/typing/engines/smirnoff/forcefield.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openforcefield/typing/engines/smirnoff/forcefield.py b/openforcefield/typing/engines/smirnoff/forcefield.py index fe7a72538..74f299deb 100644 --- a/openforcefield/typing/engines/smirnoff/forcefield.py +++ b/openforcefield/typing/engines/smirnoff/forcefield.py @@ -223,7 +223,7 @@ def __init__(self, parameter_io_handler_classes=None, disable_version_check=False, allow_cosmetic_attributes=False, - load_plugins=True): + load_plugins=False): """Create a new :class:`ForceField` object from one or more SMIRNOFF parameter definition files. Parameters From e7e72ddaf6cb0a5bcc693a538fe69b8b297b921f Mon Sep 17 00:00:00 2001 From: Jeff Wagner Date: Fri, 19 Jun 2020 15:42:25 -0700 Subject: [PATCH 07/11] Update openforcefield/tests/test_parameter_plugins.py Co-authored-by: Matt Thompson --- openforcefield/tests/test_parameter_plugins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openforcefield/tests/test_parameter_plugins.py b/openforcefield/tests/test_parameter_plugins.py index 452d378ff..c91324171 100644 --- a/openforcefield/tests/test_parameter_plugins.py +++ b/openforcefield/tests/test_parameter_plugins.py @@ -1,5 +1,5 @@ """ -Test classes and function in module openforcefield.typing.engines.smirnoff.parameters.plugins +Test classes and function in module openforcefield.typing.engines.smirnoff.plugins """ import pkg_resources import pytest From 36fde3fab3c0c5ab52c80805ea4f7f60e3a6a397 Mon Sep 17 00:00:00 2001 From: Jeff Wagner Date: Fri, 19 Jun 2020 15:42:34 -0700 Subject: [PATCH 08/11] Update openforcefield/typing/engines/smirnoff/forcefield.py --- openforcefield/typing/engines/smirnoff/forcefield.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openforcefield/typing/engines/smirnoff/forcefield.py b/openforcefield/typing/engines/smirnoff/forcefield.py index 74f299deb..7cdc81078 100644 --- a/openforcefield/typing/engines/smirnoff/forcefield.py +++ b/openforcefield/typing/engines/smirnoff/forcefield.py @@ -247,7 +247,7 @@ def __init__(self, This option is primarily intended for forcefield development. allow_cosmetic_attributes : bool, optional. Default = False Whether to retain non-spec kwargs from data sources. - load_plugins: bool, optional. Default = True + load_plugins: bool, optional. Default = False Whether to load ``ParameterHandler`` classes which have been registered by installed plugins. From de297836bd97c767946b29becfabfe7b4a0e4dd4 Mon Sep 17 00:00:00 2001 From: Jeff Wagner Date: Fri, 19 Jun 2020 15:42:42 -0700 Subject: [PATCH 09/11] Update openforcefield/typing/engines/smirnoff/plugins.py --- openforcefield/typing/engines/smirnoff/plugins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openforcefield/typing/engines/smirnoff/plugins.py b/openforcefield/typing/engines/smirnoff/plugins.py index 3fdb7cd2f..d0dd0ece5 100644 --- a/openforcefield/typing/engines/smirnoff/plugins.py +++ b/openforcefield/typing/engines/smirnoff/plugins.py @@ -1,4 +1,4 @@ -"""This module defines functions for loading parameter handler and parser clases which +"""This module defines functions for loading parameter handler and parser classes which have been registered through the `entrypoint plugin system `_. From 74e47ec0b67330d3e650efc072197e72ff7400c3 Mon Sep 17 00:00:00 2001 From: j-wags Date: Fri, 19 Jun 2020 15:48:57 -0700 Subject: [PATCH 10/11] change ph test now that load_plugins=False by default --- openforcefield/tests/test_parameter_plugins.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openforcefield/tests/test_parameter_plugins.py b/openforcefield/tests/test_parameter_plugins.py index c91324171..7056b72be 100644 --- a/openforcefield/tests/test_parameter_plugins.py +++ b/openforcefield/tests/test_parameter_plugins.py @@ -73,14 +73,14 @@ def test_force_field_custom_handler(mock_entry_point_plugins): # An exception should be raised when plugins aren't allowed. with pytest.raises(KeyError) as error_info: - ForceField(force_field_contents, load_plugins=False) + ForceField(force_field_contents) assert ( "Cannot find a registered parameter handler class for tag 'CustomHandler'" in error_info.value.args[0] ) # Otherwise the FF should be created as expected. - force_field = ForceField(force_field_contents) + force_field = ForceField(force_field_contents, load_plugins=True) parameter_handler = force_field.get_parameter_handler("CustomHandler") assert parameter_handler is not None From 4878e00cc5a2313a66dfb47b40b22d229ac12430 Mon Sep 17 00:00:00 2001 From: SimonBoothroyd Date: Fri, 19 Jun 2020 19:52:36 -0600 Subject: [PATCH 11/11] Prevent the mock plugin from being prematurely loaded. --- .github/workflows/CI.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index cf7c1e954..fa37df7f0 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -100,7 +100,8 @@ jobs: run: | . devtools/gh-actions/initialize_conda.sh conda activate test - PYTEST_ARGS="-v --ignore=utilities --ignore=examples/deprecated --nbval-lax" + PYTEST_ARGS="-v --ignore=utilities --ignore=examples/deprecated" + PYTEST_ARGS+=" --ignore=openforcefield/tests/plugins.py --nbval-lax" PYTEST_ARGS+=" --cov=openforcefield --cov-report=xml --cov-config=setup.cfg" if [[ "$RDKIT" == true && "$OPENEYE" == true ]]; then PYTEST_ARGS+=" --ignore=docs --ignore=devtools --doctest-modules"