Merge branch 'main' of github.com:openpathsampling/openpathsampling-cli into wizard-extra-help

Author: dwhswenson

paths_cli/commands/wizard.py

@@ -9,9 +9,9 @@
 )
 def wizard():  # no-cov
     register_installed_plugins()
-    # breakpoint()
     TWO_STATE_TPS_WIZARD.run_wizard()
 
+
 PLUGIN = OPSCommandPlugin(
     command=wizard,
     section="Simulation setup",

paths_cli/tests/compiling/test_volumes.py

@@ -5,7 +5,7 @@
 import yaml
 import numpy as np
 import openpathsampling as paths
-from  openpathsampling.experimental.storage.collective_variables import \
+from openpathsampling.experimental.storage.collective_variables import \
         CoordinateFunctionCV
 from openpathsampling.tests.test_helpers import make_1d_traj
 

paths_cli/tests/wizard/test_cvs.py

@@ -8,7 +8,7 @@
 
 from paths_cli.wizard.cvs import (
     TOPOLOGY_CV_PREREQ, _get_atom_indices, MDTRAJ_DISTANCE, MDTRAJ_ANGLE,
-    MDTRAJ_DIHEDRAL, COORDINATE_CV, CV_PLUGIN
+    MDTRAJ_DIHEDRAL, COORDINATE_CV
 )
 
 import openpathsampling as paths

paths_cli/tests/wizard/test_helper.py

@@ -2,7 +2,6 @@
 
 from paths_cli.wizard.helper import *
 from paths_cli.wizard.helper import _LONG_EVAL_HELP
-from paths_cli.tests.wizard.mock_wizard import mock_wizard
 
 def test_raise_quit():
     with pytest.raises(QuitWizard):
@@ -44,7 +43,7 @@ def setup(self):
         self.helper = Helper(help_func=lambda s, ctx: s)
 
     def test_help_string(self):
-        # if the helper "function" is a string, retuwn that string whether
+        # if the helper "function" is a string, return that string whether
         # or not additional arguments to help are given
         helper = Helper(help_func="a string")
         assert helper("?") == "a string"

paths_cli/tests/wizard/test_parameters.py

@@ -179,7 +179,7 @@ def test_call(self, objs):
                   'more': ['1', '2']}[objs]
 
         wrappers = {"name" + i: self.Wrapper(i*2) for i in ['1', '2', '3']}
-        items = [(key, val) for key, val in list(wrappers.items())[:n_objs]]
+        items = list(wrappers.items())[:n_objs]
 
         wiz = mock_wizard(inputs)
         wiz.store = dict(items)

paths_cli/tests/wizard/test_plugin_classes.py

@@ -1,7 +1,7 @@
 import pytest
 from unittest import mock
 from paths_cli.wizard.plugin_classes import *
-from paths_cli.tests.wizard.test_helper import mock_wizard
+from paths_cli.tests.wizard.mock_wizard import mock_wizard
 from paths_cli.wizard.standard_categories import Category
 from paths_cli.wizard.parameters import WizardParameter, ProxyParameter
 
@@ -248,7 +248,6 @@ def test_bad_arg(self, input_type):
 
 class TestWrapCategory:
     def setup(self):
-        # TODO: finishing this now
         self.wrapper = WrapCategory("foo", "ask foo", intro="intro_foo")
         self.plugin_no_format = WizardObjectPlugin(
             name="bar",
@@ -291,9 +290,12 @@ def test_register_plugin(self):
         self.wrapper.register_plugin(self.plugin_no_format)
         assert len(self.wrapper.choices) == 1
         assert self.wrapper.choices['bar'] == self.plugin_no_format
-        # TODO: what is the desired behavior if more that one plugin tries
-        # to register with the same name? override or error? currently
-        # overrides, but that should not be considered API
+
+    def test_register_plugin_duplicate(self):
+        self.wrapper.choices['bar'] = self.plugin_no_format
+        with pytest.raises(WizardObjectPluginRegistrationError,
+                           match="already been registered"):
+            self.wrapper.register_plugin(self.plugin_format)
 
     @pytest.mark.parametrize('input_type', ['method', 'format', 'string'])
     def test_get_ask(self, input_type):

paths_cli/tests/wizard/test_plugin_registration.py

@@ -2,7 +2,7 @@
 from unittest import mock
 from paths_cli.wizard.plugin_registration import *
 from paths_cli.wizard.plugin_registration import _register_category_plugin
-from paths_cli.tests.wizard.test_helper import mock_wizard
+from paths_cli.tests.wizard.mock_wizard import mock_wizard
 from paths_cli.plugin_management import OPSPlugin
 from paths_cli.wizard.plugin_classes import WrapCategory, WizardObjectPlugin
 
@@ -13,7 +13,7 @@ def _simple_func(wizard, context=None):
     return 10
 
 class MockPlugin(OPSPlugin):
-    def __init__(self, name='foo', requires_ops=(1,0), requires_cli=(0,3)):
+    def __init__(self, name='foo', requires_ops=(1, 0), requires_cli=(0, 3)):
         super().__init__(requires_ops, requires_cli)
 
     def __call__(self, wizard, context):

paths_cli/tests/wizard/test_volumes.py

@@ -5,7 +5,7 @@
 
 from paths_cli.wizard.volumes import (
     INTERSECTION_VOLUME_PLUGIN, UNION_VOLUME_PLUGIN, NEGATED_VOLUME_PLUGIN,
-    CV_DEFINED_VOLUME_PLUGIN, VOLUMES_PLUGIN, volume_intro, _VOL_DESC,
+    CV_DEFINED_VOLUME_PLUGIN, volume_intro, _VOL_DESC,
     volume_ask
 )
 

paths_cli/wizard/openmm.py

@@ -78,6 +78,5 @@
 if __name__ == "__main__":
     from paths_cli.wizard.wizard import Wizard
     wizard = Wizard([])
-    # engine = openmm_builder(wizard)
     engine = OPENMM_PLUGIN(wizard)
     print(engine)

paths_cli/wizard/plugin_classes.py

@@ -5,6 +5,11 @@
 from paths_cli.wizard.parameters import WizardParameter
 from paths_cli.wizard.helper import Helper
 
+
+class WizardObjectPluginRegistrationError(Exception):
+    pass
+
+
 _PLUGIN_DOCSTRING = """
     requires_ops : Tuple[int, int]
         version of OpenPathSampling required for this plugin
@@ -39,7 +44,7 @@ class LoadFromOPS(OPSPlugin):
         can be found
     """ + _PLUGIN_DOCSTRING
     def __init__(self, category, *, obj_name=None, store_name=None,
-                 requires_ops=(1,0), requires_cli=(0,3)):
+                 requires_ops=(1, 0), requires_cli=(0, 3)):
         super().__init__(requires_ops, requires_cli)
         self.category = category
         self.name = "Load existing from OPS file"
@@ -114,7 +119,7 @@ class WizardObjectPlugin(OPSPlugin):
     """ + _WIZARD_KWONLY
     def __init__(self, name, category, builder, *, prerequisite=None,
                  intro=None, description=None, summary=None,
-                 requires_ops=(1,0), requires_cli=(0,3)):
+                 requires_ops=(1, 0), requires_cli=(0, 3)):
         super().__init__(requires_ops, requires_cli)
         self.name = name
         self.category = category
@@ -207,7 +212,7 @@ def __init__(self, name, category, parameters, builder, *,
     @classmethod
     def from_proxies(cls, name, category, parameters, compiler_plugin, *,
                      prerequisite=None, intro=None, description=None,
-                     summary=None, requires_ops=(1,0), requires_cli=(0,3)):
+                     summary=None, requires_ops=(1, 0), requires_cli=(0, 3)):
         """
         Create plugin from proxy parameters and existing compiler plugin.
 
@@ -245,7 +250,6 @@ def _build(self, wizard, prereqs):
         context = {'obj_dict': dct}
         for param in self.parameters:
             dct[param.name] = param(wizard, context)
-        # dct.update({p.name: p(wizard) for p in self.parameters})
         result = self.build_func(**dct)
         return result
 
@@ -327,7 +331,7 @@ class WrapCategory(OPSPlugin):
 
     """
     def __init__(self, name, ask, helper=None, *, intro=None,
-                 set_context=None, requires_ops=(1,0), requires_cli=(0,3)):
+                 set_context=None, requires_ops=(1, 0), requires_cli=(0, 3)):
         super().__init__(requires_ops, requires_cli)
         self.name = name
         if isinstance(intro, str):
@@ -361,6 +365,11 @@ def register_plugin(self, plugin):
         plugin : :class:`.WizardObjectPlugin`
             the plugin to register
         """
+        if plugin.name in self.choices:
+            raise WizardObjectPluginRegistrationError(
+                f"A plugin named '{plugin.name}' has already been "
+                f"registered with the category '{self.name}'"
+            )
         self.choices[plugin.name] = plugin
 
     def _get_intro(self, wizard, context):