From ea6427481315004e89257e7b33b2b9d8b267680c Mon Sep 17 00:00:00 2001 From: "David W.H. Swenson" Date: Wed, 16 Feb 2022 06:46:15 -0500 Subject: [PATCH] CLI Skeleton (#49) * Initial skeleton for the CLI Currently requires the OPS CLI to be installed as well; next steps: 1. Add parameter core classes to infrastructure 2. Fully separate infrastructure into its own package * switch to using plugcli * tests for CLI * helps if you add the tests... * pep8 and deps cleanup * add test_plugins --- .github/workflows/ci.yaml | 2 +- environment.yml | 2 + openfecli/__init__.py | 5 ++ openfecli/cli.py | 38 ++++++++++ openfecli/commands/README.md | 128 ++++++++++++++++++++++++++++++++ openfecli/commands/echo.py | 36 +++++++++ openfecli/plugins.py | 12 +++ openfecli/tests/__init__.py | 0 openfecli/tests/test_cli.py | 38 ++++++++++ openfecli/tests/test_plugins.py | 25 +++++++ setup.cfg | 6 ++ 11 files changed, 291 insertions(+), 1 deletion(-) create mode 100644 openfecli/__init__.py create mode 100644 openfecli/cli.py create mode 100644 openfecli/commands/README.md create mode 100644 openfecli/commands/echo.py create mode 100644 openfecli/plugins.py create mode 100644 openfecli/tests/__init__.py create mode 100644 openfecli/tests/test_cli.py create mode 100644 openfecli/tests/test_plugins.py diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 0a80cfb9..c82b6e37 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -60,7 +60,7 @@ jobs: - name: "Run tests" run: | - pytest -n 2 -v --cov=openfe --cov-report=xml openfe/tests/ + pytest -n 2 -v --cov=openfe --cov=openfecli --cov-report=xml - name: codecov if: ${{ github.repository == 'OpenFreeEnergy/openfe' diff --git a/environment.yml b/environment.yml index 0e45c590..e2f02c02 100644 --- a/environment.yml +++ b/environment.yml @@ -10,5 +10,7 @@ dependencies: - pytest-xdist - pytest-cov - coverage + - click - pip: - git+https://github.com/OpenFreeEnergy/Lomap@main + - plugcli diff --git a/openfecli/__init__.py b/openfecli/__init__.py new file mode 100644 index 00000000..f8264d2c --- /dev/null +++ b/openfecli/__init__.py @@ -0,0 +1,5 @@ +# This code is part of OpenFE and is licensed under the MIT license. +# For details, see https://github.com/OpenFreeEnergy/openfe + +from .plugins import OFECommandPlugin +from . import commands diff --git a/openfecli/cli.py b/openfecli/cli.py new file mode 100644 index 00000000..5b3182bc --- /dev/null +++ b/openfecli/cli.py @@ -0,0 +1,38 @@ +# This code is part of OpenFE and is licensed under the MIT license. +# For details, see https://github.com/OpenFreeEnergy/openfe + +import pathlib + +import click + +from plugcli.cli import CLI, CONTEXT_SETTINGS +from plugcli.plugin_management import FilePluginLoader +from .plugins import OFECommandPlugin + + +class OpenFECLI(CLI): + COMMAND_SECTIONS = ["Setup", "Simulation", "Orchestration", "Analysis"] + + def get_installed_plugins(self): + commands = str(pathlib.Path(__file__).parent.resolve() / "commands") + loader = FilePluginLoader(commands, OFECommandPlugin) + return list(loader()) + + +_MAIN_HELP = """ +This is the command line tool to provide easy access to functionality from +the OpenFE Python library. +""" + + +@click.command(cls=OpenFECLI, name="openfe", help=_MAIN_HELP, + context_settings=CONTEXT_SETTINGS) +def main(): + # currently empty: we can add options at the openfe level (as opposed to + # subcommands) by adding click options here. Subcommand runs after this + # is the processed. + pass + + +if __name__ == "__main__": # -no-cov- (useful in debugging) + main() diff --git a/openfecli/commands/README.md b/openfecli/commands/README.md new file mode 100644 index 00000000..4a59c111 --- /dev/null +++ b/openfecli/commands/README.md @@ -0,0 +1,128 @@ +# Contributing CLI Subcommands + +Adding a new subcommand to the `openfe` CLI is pretty straightforward, but +there are some best practices that will make your contribution easier to +maintain. + +## How the CLI finds subcommands + +Subcommands are registered with the CLI based on the existence of an instance +of `CommandPlugin` in modules located in particular directories or namespaces. +This means that after you create the command function, you need to wrap it in a +`CommandPlugin`, which must be assigned to a variable name. The variable name +itself is unimportant (I usually use `PLUGIN`). It's perfectly fine to include +more than one plugin in the same file, but the each must have a different +variable name. + +The allowed locations for command plugins may change, but currently includes: + +* modules located in the namespace `openfecli.commands` + +When contributing to the core CLI, all you need to do is add your subcommand +module to the `openfecli/commands/` directory, and the CLI should register it +automatically. + +## Best practices + +### The CLI should be a thin wrapper around the library + +The intent of the CLI is to provide convenient ways of accomplishing things +that can also be accomplished with the core library. This means that CLI +commands should be thin wrappers that either just call a method from the core +library, or run a very simple workflow based on methods from the core library. + +If you find that your CLI command starts to have some more complex logic, this +probably means that some of that logic would be beneficial to users of the +library as well. Consider moving that code into the core library. + +This also implies that we can split any CLI subcommand into two stages: + +1. Convert from user input to objects that have meaning to the library. +2. Run some code as if we were users of the library, with no reference to the + fact that the inputs came from the command line. + +### Divide the subcommand module into three components + +The recommended way of structuring a subcommand module is to split it into +three parts (where `command` is replaced by the name of your subcommand): + +1. `command`: The command method, which is decorated by `@click.command`. The + purpose of this method is to convert user input to objects that can be used + by the core library. Then it calls the `command_main` method. +2. `command_main`: The workflow method, which is written using code from the + underlying library, with no reference to the fact that this is part of the + CLI. This typically contains a very simple workflow script. Although the + output from this process is usually saved to some output file as part of the + script in `command_main`, the best practice is to also return the result of + this method. The `command` method will ignore this return value, but + returning it makes it so that the `command_main` method can be reused in + other CLI commands to create more complex workflows. +3. `PLUGIN`: a `CommandPlugin` instance, which wraps the `command` object with + metadata about the subcommand, such as which help section to display it in, + and which versions of the library and CLI the plugin is compatible with. + +As an example, here's a rough skeleton for a subcommand called `my_command` +(imports excluded) + +```python +@click.command("my_command", short_help="This is my command") +... # add decorators for arguments/options +def my_command(...): # input params are based on arguments options + """Docstring here is the help given by ``openfe my-command --help``""" + ... # do whatever you need to convert the user input to library objects + my_command_main(...) # takes library objects + +def my_command_main(...): # takes library objects + ... # run some simple library code + return result + +PLUGIN = CommandPlugin( + command=my_command, + section="My Section", + requires_lib=(1, 0), + requires_cli=(1, 0) +) +``` + +### Use reusable subcommand arguments/options + +In `click`, command-line arguments and options are declared by attaching +decorators for each option to a method. The method must then take parameters +based on the option name as specified by the decorator. + +Because of this, it is straightforward to create an object associated with a +given input option/argument, which contains details such as the help string and +even a method to get a library object from the user input string. + +The best practice is to create this object outside a given subcommand, and then +reuse it between different subcommands. This ensures that the user sees +consistency in the interface and behavior between different CLI subcommands. + +Details on how we'll do this in OpenFE are still being developed. + +### Delay slow imports + +Usually in Python, we put all imports at the top of a file. That is the best +practice for libraries and scripts, because it makes it easy for a developer to +find dependencies, and helps prevent developers from repeating import +statements. + +However, when dealing with a CLI script like this, it's important to remember +that some user interactions, such as subcommand autocomplete or enquiring about the +CLI with `--version` or `--help`, will also run any top-level imports. If +imports are slow, then these user-facing interactions will be slow. + +Because of this, the best practice when writing CLI subcommands is to move slow +imports inside the method that needs them. + +### Testing your subcommand + +Dividing the subcommand as recommended above facilitates testing. When testing +the `command` method itself, mock out the `command_main`, and use tools within +`click` to mock the user command inputs. The purpose of testing the `command` +method is to ensure that you correctly convert from user input to library object. + +The purpose of testing the `command_main` method is to ensure that integration +with the library works. If this is truly a thin wrapper (and with the +assumption that the core library is thoroughly tested), then a smoke test may +be sufficient for `command_main`. diff --git a/openfecli/commands/echo.py b/openfecli/commands/echo.py new file mode 100644 index 00000000..9f72e2a2 --- /dev/null +++ b/openfecli/commands/echo.py @@ -0,0 +1,36 @@ +# THIS IS A TEMPORARY FILE TO ILLUSTRATE HOW ONE WRITES A SUBCOMMAND PLUGIN. +# DELETE ONCE WE HAVE A FEW REAL EXAMPLES! + +import click +from openfecli import OFECommandPlugin + + +@click.command( + "echo", + short_help="This shows up in ``openfe --help``" +) +def echo(): + """ + This is the longer help statement that shows up when you get help for an + individual command, e.g., with ``openfe echo --help``. + """ + # the code here serves to convert user input to objects that would be + # run by library code. In general, this should be done with a ``get`` + # method attached to the input decorators + echo_main() + + +def echo_main(): + # the code here does the actual workflow in the library. This will tend + # to be a very small code + print("foo") + + +PLUGIN = OFECommandPlugin( + command=echo, + section="Simulation", + requires_ofe=(0, 0, 1) +) + +if __name__ == "__main__": + echo() diff --git a/openfecli/plugins.py b/openfecli/plugins.py new file mode 100644 index 00000000..16aa17d4 --- /dev/null +++ b/openfecli/plugins.py @@ -0,0 +1,12 @@ +# This code is part of OpenFE and is licensed under the MIT license. +# For details, see https://github.com/OpenFreeEnergy/openfe + +from plugcli.plugin_management import CommandPlugin + + +class OFECommandPlugin(CommandPlugin): + def __init__(self, command, section, requires_ofe): + super().__init__(command=command, + section=section, + requires_lib=requires_ofe, + requires_cli=requires_ofe) diff --git a/openfecli/tests/__init__.py b/openfecli/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/openfecli/tests/test_cli.py b/openfecli/tests/test_cli.py new file mode 100644 index 00000000..12a3b153 --- /dev/null +++ b/openfecli/tests/test_cli.py @@ -0,0 +1,38 @@ +import pytest + +import click.testing + +from openfecli.cli import OpenFECLI, main +from openfecli.plugins import OFECommandPlugin + + +class TestCLI: + def setup(self): + self.cli = OpenFECLI() + + def test_invoke(self): + runner = click.testing.CliRunner() + with runner.isolated_filesystem(): + # isolated_filesystem is overkill here, but good practice for + # testing with CliRunner + result = runner.invoke(main, ["-h"]) + assert result.exit_code == 0 + assert "Usage: openfe" in result.output + + def test_command_sections(self): + # This test does not ensure the order of the sections, and does not + # prevent other sections from being added later. It only ensures + # that the main 4 sections continue to exist. + included = ["Setup", "Simulation", "Orchestration", "Analysis"] + for sec in included: + assert sec in self.cli.COMMAND_SECTIONS + + def test_get_installed_plugins(self): + # Test that we correctly load some plugins. This test only ensures + # that some plugins are loaded; it currently does nothing to ensure + # the identity of the specific plugins. + plugins = self.cli.get_installed_plugins() + for plugin in plugins: + assert isinstance(plugin, OFECommandPlugin) + + assert len(plugins) > 0 diff --git a/openfecli/tests/test_plugins.py b/openfecli/tests/test_plugins.py new file mode 100644 index 00000000..098e6127 --- /dev/null +++ b/openfecli/tests/test_plugins.py @@ -0,0 +1,25 @@ +import click +from openfecli.plugins import OFECommandPlugin + + +@click.command("fake") +def fake(): + pass # -no-cov- a fake placeholder click subcommand + + +class TestOFECommandPlugin: + def setup(self): + self.plugin = OFECommandPlugin( + command=fake, + section="Some Section", + requires_ofe=(0, 0, 1) + ) + + def test_plugin_setup(self): + assert self.plugin.command is fake + assert isinstance(self.plugin.command, click.Command) + assert self.plugin.section == "Some Section" + assert self.plugin.requires_lib == self.plugin.requires_cli + assert self.plugin.requires_lib == (0, 0, 1) + assert self.plugin.requires_cli == (0, 0, 1) + diff --git a/setup.cfg b/setup.cfg index c9380a44..02ec8a78 100644 --- a/setup.cfg +++ b/setup.cfg @@ -24,6 +24,8 @@ python_requires = >= 3.8 install_requires = numpy networkx + click + plugcli include_package_data = True packages = find: @@ -36,6 +38,10 @@ test = pytest pytest-xdist +[options.entry_points] +console_scripts = + openfe = openfecli.cli:main + [bdist_wheel] universal = 1