-
Notifications
You must be signed in to change notification settings - Fork 227
API: Add the aiida.engine.processes.control
module
#5630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
API: Add the aiida.engine.processes.control
module
#5630
Conversation
e1e6506
to
cd0a626
Compare
c18d524
to
dd2e88d
Compare
These utilities are useful not only for the CLI but also in user applications, such as notebooks or in plugins. It makes more sense to have these in `aiida.tools`. The original locations import the new modules to keep backwards compatibility and a deprecation warning is added to the `aiida.cmdline` modules. The choices and default of the `--project` option of the `process list` had to be made indirect because of the move. The reason is that they rely on the `CalculationQueryBuilder` class, which now has moved to the `aiida.tools` module. This means that it should be imported on the top level of the `aiida.cmdline`, however, this is not permissible as the load time of this is significant and it will slow down tab-completion, which will have to load the cmdline module. The work around is to make the import of the class indirect by using the `LazyChoice` class as the type of the option and by making the default a lambda that calls a function that performs the actual import.
dd2e88d
to
b83ecb8
Compare
Docs build is failing but I haven't been able to fix it yet. The problem is coming from the type hints. Adding a rule to the ignore nitpicks didn't seem to help. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thanks a lot @sphuber !
Splitting the move between files into a separate commit made the review much more straightforward.
I don't have any major comments, just a few suggestions
looks good cheers 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing the comments; good to go from my side as well.
This module abstracts functionality that was implemented directly in the `aiida.cmdline.commands.cmd_process` module that allows to interact with active processes. By extracting it to a separate module, this functionality can now also be easily used by users from interfaces other than the CLI, such as the shell, or a notebook.
8fc9d14
to
76561cf
Compare
Fixes #5258
This module abstracts functionality that was implemented directly in the
aiida.cmdline.commands.cmd_process
module that allows to interact withactive processes. By extracting it to a separate module, this
functionality can now also be easily used by users from interfaces other
than the CLI, such as the shell, or a notebook.