* feat: support `openfe gather` for septop (#1638)
* copy septop analysis notebook over
* add todos
* add plan and test
* format
* add failing tests
* comment out secondary tests
* add cli tmp
* clean up imports
* fix type hints in gather.py
* test passes for gather raw
* test passes for ddg
* tests pass for dg
* pull more error handling into septop
* condense code a bit
* MBAR uncertainty
* remove planning stub
* Revert "MBAR uncertainty"
This reverts commit bf32aa3c6e.
* format
* test with tolerances
* precommit format
* remove accidentally committed file
* Apply suggestions from code review
Co-authored-by: Hannah Baumann <43765638+hannahbaumann@users.noreply.github.com>
---------
Co-authored-by: Hannah Baumann <43765638+hannahbaumann@users.noreply.github.com>
* feat: add abfe gathering support first draft (#1686)
* add abfe MVP with tests
* Apply suggestions from code review
Co-authored-by: Hannah Baumann <43765638+hannahbaumann@users.noreply.github.com>
* fix incorrect var name
* fix abfe legs extraction
* fix merge bug
* remove unused import
---------
Co-authored-by: Hannah Baumann <43765638+hannahbaumann@users.noreply.github.com>
* gather: fix septop rounding (#1696)
* only apply precision rounding at the end
* clean up code
* gather: update names parsing (septop and abfe) (#1698)
* update abfe names parsing
* update septop names parsing
* add todo
* get names from alchemical_components
* remove todo
* gather: refactor/unify rounding behavior (#1697)
* format septop outputs with format_df_with_precision
* only apply precision rounding at the end
* update gather abfe formatting to use format_df_with_precision
* remove unused import
* switch back to checking final outputs for abfes
* clean up code
* add rounding for dg mle
* simplify code
* gather: unify code structure between rbfe, septop, & abfe (#1700)
* format septop outputs with format_df_with_precision
* only apply precision rounding at the end
* update gather abfe formatting to use format_df_with_precision
* remove unused import
* switch back to checking final outputs for abfes
* clean up code
* add rounding for dg mle
* simplify code
* make gather_septop.py code more similar to gather.py
* make gather_abfe.py code more similar to gather.py
* remove unused function
* add stdout message for experimental gathering support (#1703)
* add warnings for experimental gathering
* use click to make output look nicer, not a true warning
* add warning text
* make yellow
* add tests
* remove unused code
* add news item
* speed up names gathering
* names should always return str
* add single repeat tests for abfe and septop
* remove unused imports
* make `gather septop/abfe` output headers accurately reflect error type (#1712)
* gather septop/abfe output headers accurately reflect error type
* split into func
* label all uncertainty columns more specifically
* add docstring
* update test data
* update error calculation
---------
Co-authored-by: Hannah Baumann <43765638+hannahbaumann@users.noreply.github.com>
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:
- Convert from user input to objects that have meaning to the library.
- 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):
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 thecommand_mainmethod.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 incommand_main, the best practice is to also return the result of this method. Thecommandmethod will ignore this return value, but returning it makes it so that thecommand_mainmethod can be reused in other CLI commands to create more complex workflows.PLUGIN: aCommandPlugininstance, which wraps thecommandobject 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)
@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.