Skip to content

Conversation

@dwhswenson
Copy link
Member

@dwhswenson dwhswenson commented Jul 24, 2021

This PR introduces a new plugin infrastructure for the OPS CLI. This is a major API break, and all externally-developed plugins will need to update for it.

Fortunately, the update procedure is simple: Previously, plugins had to be one per module (file), and the modules needed to define the variables CLI (to label the command), SECTION (the help section to use), and optionally REQUIRES_OPS (the minimum OPS version).

The new plugins require importing paths_cli.OPSCommandPlugin, and then creating an OPSCommandPlugin instance, which takes the required variables command and section, and the optional variables requires_ops and requires_cli (which defines the minimum CLI version.) Note that this OPSCommandPlugin instance must be assigned to a name in the module (it needs to be listed in the module's vars.) I happen to choose the name PLUGIN in the examples, but the specific name doesn't matter.

For examples on how to do the update, see changes to files in the commands directory in this PR.

The major advantages of the new plugin system are:

  1. It is possible to define more than one plugin in a given file.
  2. The new approach is not narrowly focused on subcommands of the CLI. The plugin infrastructure could be used for plugins for other tools, such as in Simulation Setup Wizard #41 or in compile command: Simulation setup with YAML or JSON #43.

For developers who want to support for OPS CLI 0.3 (where this will be included) and earlier versions, the two methods can be used simultaneously, but both methods must be included in order for plugins to work with all versions of the CLI (i.e., unmodified old plugins will not load with new versions of the CLI).

Tasks:

  • Code
  • Tests
  • Migrate commands
  • Migrate miscellaneous (examples for writing plugins; plugins used in testing)
  • Update docstrings on plugin management tools
  • Update docs on writing plugins

@dwhswenson dwhswenson added enhancement New feature or request API break labels Jul 24, 2021
@codecov
Copy link

codecov bot commented Jul 24, 2021

Codecov Report

Merging #48 (8ec5650) into main (728db4d) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 8ec5650 differs from pull request most recent head 4771349. Consider uploading reports for the commit 4771349 to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##              main       #48   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          599       613   +14     
=========================================
+ Hits           599       613   +14     
Impacted Files Coverage Δ
paths_cli/param_core.py 100.00% <ø> (ø)
paths_cli/__init__.py 100.00% <100.00%> (ø)
paths_cli/cli.py 100.00% <100.00%> (ø)
paths_cli/commands/append.py 100.00% <100.00%> (ø)
paths_cli/commands/contents.py 100.00% <100.00%> (ø)
paths_cli/commands/equilibrate.py 100.00% <100.00%> (ø)
paths_cli/commands/md.py 100.00% <100.00%> (ø)
paths_cli/commands/pathsampling.py 100.00% <100.00%> (ø)
paths_cli/commands/visit_all.py 100.00% <100.00%> (ø)
paths_cli/plugin_management.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 728db4d...4771349. Read the comment docs.

@dwhswenson
Copy link
Member Author

This is ready for review and comment. I will leave it open for at least 48 hours, merging no earlier than Tue 27 Jul 05:00 GMT (01:00 my local).

Copy link
Member

@sroet sroet left a comment

Choose a reason for hiding this comment

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

Not knowledgeable enough to do a proper review, but these were the things that stood out to me. Feel free to ignore

Co-authored-by: Sander Roet <sanderroet@hotmail.com>
@dwhswenson dwhswenson merged commit ba3e668 into openpathsampling:main Jul 27, 2021
@dwhswenson dwhswenson deleted the new_plugins branch July 27, 2021 17:18
@dwhswenson dwhswenson mentioned this pull request Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API break enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants