From 26df1717a0d31529bc0d5f767b4996b763b117bb Mon Sep 17 00:00:00 2001 From: Tristan Kuehn Date: Mon, 6 Mar 2023 16:43:57 -0500 Subject: [PATCH 1/8] Implement first pass at simple plugins --- snakebids/app.py | 24 +++++++++++++++++++++++- snakebids/tests/test_app.py | 7 +++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/snakebids/app.py b/snakebids/app.py index b1f16a3e..33358a2f 100644 --- a/snakebids/app.py +++ b/snakebids/app.py @@ -1,10 +1,12 @@ """Tools to generate a Snakemake-based BIDS app.""" +from __future__ import annotations import argparse import logging import sys +from collections.abc import Sequence from pathlib import Path -from typing import Any, Dict, List, Optional +from typing import Any, Callable, Dict, List, Optional import attr import boutiques.creator as bc @@ -111,6 +113,22 @@ class SnakeBidsApp: takes_self=True, ) args: Optional[SnakebidsArgs] = None + plugins: list[Callable[[SnakeBidsApp], None]] = attr.Factory(list) + + def add_plugins(self, plugins: Sequence[Callable[[SnakeBidsApp], None]]): + """Supply list of methods to be called after cli parsing + + Each callable in ``plugins`` should take, as a single argument, a + reference to the ``SnakeBidsApp``, and should not return anything. + Plugins may perform any arbitrary side effects, including validation, + optimization, other other enhancements to the snakebids app. + + CLI parameters may be read from ``SnakeBidsApp.config``. Plugins + are responsible for documenting what properties they expect to find + in the config. + """ + # pylint: disable=no-member + self.plugins.extend(plugins) def run_snakemake(self): """Run snakemake with that config. @@ -187,6 +205,10 @@ def run_snakemake(self): new_config_file = args.outputdir / self.configfile_path self.config["root"] = "" + # pylint: disable=not-an-iterable + for plugin in self.plugins: + plugin(self) + # Write the config file write_config_file( config_file=new_config_file, diff --git a/snakebids/tests/test_app.py b/snakebids/tests/test_app.py index 63b90afb..d7aa01f6 100644 --- a/snakebids/tests/test_app.py +++ b/snakebids/tests/test_app.py @@ -117,12 +117,19 @@ def test_runs_in_correct_mode( reset_db=True, ) + def plugin(my_app): + my_app.foo = "bar" + + app.add_plugins([plugin]) + try: app.run_snakemake() except SystemExit as e: print("System exited prematurely") print(e) + assert app.foo == "bar" + # First condition: outputdir is an arbitrary path if root not in ["app", "app/results"] or (root == "app" and tail): cwd = outputdir.resolve() From e4910a51ae332111c8d81cd157fa2c0e566e604e Mon Sep 17 00:00:00 2001 From: Tristan Kuehn Date: Tue, 7 Mar 2023 15:41:40 -0500 Subject: [PATCH 2/8] Allow plugins to return modified SnakeBidsApps --- snakebids/app.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/snakebids/app.py b/snakebids/app.py index 33358a2f..95345f4a 100644 --- a/snakebids/app.py +++ b/snakebids/app.py @@ -113,9 +113,11 @@ class SnakeBidsApp: takes_self=True, ) args: Optional[SnakebidsArgs] = None - plugins: list[Callable[[SnakeBidsApp], None]] = attr.Factory(list) + plugins: list[Callable[[SnakeBidsApp], None | SnakeBidsApp]] = attr.Factory(list) - def add_plugins(self, plugins: Sequence[Callable[[SnakeBidsApp], None]]): + def add_plugins( + self, plugins: Sequence[Callable[[SnakeBidsApp], None | SnakeBidsApp]] + ): """Supply list of methods to be called after cli parsing Each callable in ``plugins`` should take, as a single argument, a @@ -205,14 +207,15 @@ def run_snakemake(self): new_config_file = args.outputdir / self.configfile_path self.config["root"] = "" + app = self # pylint: disable=not-an-iterable for plugin in self.plugins: - plugin(self) + app = plugin(app) or app # Write the config file write_config_file( config_file=new_config_file, - data=self.config, + data=app.config, force_overwrite=True, ) @@ -224,14 +227,14 @@ def run_snakemake(self): None, [ "--snakefile", - str(self.snakefile_path), + str(app.snakefile_path), "--directory", str(cwd), "--configfile", str(new_config_file.resolve()), - *self.config["snakemake_args"], - *self.config["targets_by_analysis_level"][ - self.config["analysis_level"] + *app.config["snakemake_args"], + *app.config["targets_by_analysis_level"][ + app.config["analysis_level"] ], ], ) From bc0100d0437de9ba308ca33902ffce480b427dea Mon Sep 17 00:00:00 2001 From: Tristan Kuehn Date: Tue, 7 Mar 2023 15:43:10 -0500 Subject: [PATCH 3/8] Have SnakeBidsApp.add_plugins take an Iterable --- snakebids/app.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/snakebids/app.py b/snakebids/app.py index 95345f4a..2414966e 100644 --- a/snakebids/app.py +++ b/snakebids/app.py @@ -4,7 +4,7 @@ import argparse import logging import sys -from collections.abc import Sequence +from collections.abc import Iterable from pathlib import Path from typing import Any, Callable, Dict, List, Optional @@ -116,7 +116,7 @@ class SnakeBidsApp: plugins: list[Callable[[SnakeBidsApp], None | SnakeBidsApp]] = attr.Factory(list) def add_plugins( - self, plugins: Sequence[Callable[[SnakeBidsApp], None | SnakeBidsApp]] + self, plugins: Iterable[Callable[[SnakeBidsApp], None | SnakeBidsApp]] ): """Supply list of methods to be called after cli parsing From eb447b0b693b8cf94bcd8a1049d58edeae631f1c Mon Sep 17 00:00:00 2001 From: Tristan Kuehn Date: Tue, 7 Mar 2023 15:57:24 -0500 Subject: [PATCH 4/8] Expand on the docstring explaining plugins --- snakebids/app.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/snakebids/app.py b/snakebids/app.py index 2414966e..afb7694c 100644 --- a/snakebids/app.py +++ b/snakebids/app.py @@ -121,7 +121,15 @@ def add_plugins( """Supply list of methods to be called after cli parsing Each callable in ``plugins`` should take, as a single argument, a - reference to the ``SnakeBidsApp``, and should not return anything. + reference to the ``SnakeBidsApp``, and should return either: + + - Nothing, in which case any changes to the SnakeBidsApp need to + come from mutating it. + - A ``SnakeBidsApp``, which will be used to call Snakemake. Note + that in this case, any processing of CLI arguments and + configuration must already be handled, so it is recommended to + use ``copy.deepcopy`` to copy the original ``SnakeBidsApp``. + Plugins may perform any arbitrary side effects, including validation, optimization, other other enhancements to the snakebids app. From 232a3f672862bdfbc776447935fb25859b302232 Mon Sep 17 00:00:00 2001 From: Tristan Kuehn Date: Tue, 7 Mar 2023 16:21:23 -0500 Subject: [PATCH 5/8] First attempt at Sphinx docs --- docs/bids_app/plugins.md | 5 +++++ docs/index.md | 1 + 2 files changed, 6 insertions(+) create mode 100644 docs/bids_app/plugins.md diff --git a/docs/bids_app/plugins.md b/docs/bids_app/plugins.md new file mode 100644 index 00000000..3ee91d1b --- /dev/null +++ b/docs/bids_app/plugins.md @@ -0,0 +1,5 @@ +# Plugins + +Plugins are a Snakebids feature that allow you to add arbitrary behaviour to your Snakebids app after CLI arguments are parsed but before Snakemake is invoked. For example, you might add BIDS validation of an input dataset to your app via a plugin, so your app is only run if the input dataset is valid. + +A plugin is simply a function that takes a {class}`SnakeBidsApp ` as input and returns either a modified {class}`SnakeBidsApp ` or `None`. To add one or more plugins to your {class}`SnakeBidsApp `, use the method {func}`add_plugins `, which will typically be easiest in a `run.py` or similar entrypoint to your app. Your plugin will have access to CLI parameters via `SnakeBidsApp.config`, but you're responsible for making sure that your app provides the properties a plugin expects to find in the config. diff --git a/docs/index.md b/docs/index.md index b99997ae..b81dbbfc 100644 --- a/docs/index.md +++ b/docs/index.md @@ -39,6 +39,7 @@ bids_function/overview bids_app/overview bids_app/config bids_app/workflow +bids_app/plugins ``` ```{toctree} From 9312afe421af8156888c8d180ca1806e212041c5 Mon Sep 17 00:00:00 2001 From: Tristan Kuehn Date: Wed, 8 Mar 2023 09:12:28 -0500 Subject: [PATCH 6/8] Update add_plugins docstring --- snakebids/app.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/snakebids/app.py b/snakebids/app.py index afb7694c..79757085 100644 --- a/snakebids/app.py +++ b/snakebids/app.py @@ -118,24 +118,23 @@ class SnakeBidsApp: def add_plugins( self, plugins: Iterable[Callable[[SnakeBidsApp], None | SnakeBidsApp]] ): - """Supply list of methods to be called after cli parsing + """Supply list of methods to be called after CLI parsing. Each callable in ``plugins`` should take, as a single argument, a - reference to the ``SnakeBidsApp``, and should return either: - - - Nothing, in which case any changes to the SnakeBidsApp need to - come from mutating it. - - A ``SnakeBidsApp``, which will be used to call Snakemake. Note - that in this case, any processing of CLI arguments and - configuration must already be handled, so it is recommended to - use ``copy.deepcopy`` to copy the original ``SnakeBidsApp``. - - Plugins may perform any arbitrary side effects, including validation, - optimization, other other enhancements to the snakebids app. + reference to the ``SnakeBidsApp``. Plugins may perform any arbitrary + side effects, including updates to the config dictionary, validation + of inputs, optimization, or other enhancements to the snakebids app. CLI parameters may be read from ``SnakeBidsApp.config``. Plugins are responsible for documenting what properties they expect to find in the config. + + Every plugin should return either: + + - Nothing, in which case any changes to the SnakeBidsApp will + persist in the workflow. + - A ``SnakeBidsApp``, which will replace the existing instance, + so this option should be used with care. """ # pylint: disable=no-member self.plugins.extend(plugins) From 42826d42440b028b72ed6eda915f36ab49069331 Mon Sep 17 00:00:00 2001 From: Tristan Kuehn Date: Wed, 8 Mar 2023 10:32:40 -0500 Subject: [PATCH 7/8] Update sphinx docs --- docs/bids_app/plugins.md | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/docs/bids_app/plugins.md b/docs/bids_app/plugins.md index 3ee91d1b..a2fbfcd7 100644 --- a/docs/bids_app/plugins.md +++ b/docs/bids_app/plugins.md @@ -2,4 +2,31 @@ Plugins are a Snakebids feature that allow you to add arbitrary behaviour to your Snakebids app after CLI arguments are parsed but before Snakemake is invoked. For example, you might add BIDS validation of an input dataset to your app via a plugin, so your app is only run if the input dataset is valid. -A plugin is simply a function that takes a {class}`SnakeBidsApp ` as input and returns either a modified {class}`SnakeBidsApp ` or `None`. To add one or more plugins to your {class}`SnakeBidsApp `, use the method {func}`add_plugins `, which will typically be easiest in a `run.py` or similar entrypoint to your app. Your plugin will have access to CLI parameters via `SnakeBidsApp.config`, but you're responsible for making sure that your app provides the properties a plugin expects to find in the config. +A plugin is simply a function that takes a {class}`SnakeBidsApp ` as input and returns either a modified {class}`SnakeBidsApp ` or `None`. To add one or more plugins to your {class}`SnakeBidsApp `, use the method {func}`add_plugins `, which will typically be easiest in a `run.py` or similar entrypoint to your app. Your plugin will have access to CLI parameters (after they've been parsed) via their names in {attr}`SnakeBidsApp.config `. Any modifications to that config dictionary will be carried forward into the workflow. + +As an example, a plugin could run the [BIDS Validator](https://github.com/bids-standard/bids-validator) on the input directory like so: + +``` +import subprocess + +from snakebids.app import SnakeBidsApp + +def bids_validate(app: SnakeBidsApp) -> None: + if app.config["skip_bids_validation"]: + return + + try: + subprocess.run(["bids-validator", app.config["bids_dir"]], check=True) + except subprocess.CalledProcessError as err: + raise InvalidBidsError from err + +class InvalidBidsError(Exception): + """Error raised if an input BIDS dataset is invalid.""" + +app = SnakeBidsApp("path/to/snakebids/app") +app.add_plugins([bids_validate]) +app.run_snakemake() + +``` + +You would also want to add some logic to check if the BIDS Validator is installed and pass along the error message, but the point is that a plugin can do anything that can be handled by a Python function. From d9121d298327b86c6de29ba6153d9179d41981bb Mon Sep 17 00:00:00 2001 From: Tristan Kuehn Date: Wed, 8 Mar 2023 10:46:39 -0500 Subject: [PATCH 8/8] Factor plugins test into its own function --- snakebids/tests/test_app.py | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/snakebids/tests/test_app.py b/snakebids/tests/test_app.py index d7aa01f6..2d252a41 100644 --- a/snakebids/tests/test_app.py +++ b/snakebids/tests/test_app.py @@ -117,19 +117,12 @@ def test_runs_in_correct_mode( reset_db=True, ) - def plugin(my_app): - my_app.foo = "bar" - - app.add_plugins([plugin]) - try: app.run_snakemake() except SystemExit as e: print("System exited prematurely") print(e) - assert app.foo == "bar" - # First condition: outputdir is an arbitrary path if root not in ["app", "app/results"] or (root == "app" and tail): cwd = outputdir.resolve() @@ -169,6 +162,34 @@ def plugin(my_app): ] ) + def test_plugins(self, mocker: MockerFixture, app: SnakeBidsApp): + # Get mocks for all the io functions + self.io_mocks(mocker) + mocker.patch.object( + sn_app, + "update_config", + side_effect=lambda config, sn_args: config.update(sn_args.args_dict), + ) + output_dir = Path("app") / "results" + app.args = SnakebidsArgs( + force=False, + outputdir=output_dir, + snakemake_args=[], + args_dict={"output_dir": output_dir.resolve()}, + ) + + def plugin(my_app): + my_app.foo = "bar" + + app.add_plugins([plugin]) + try: + app.run_snakemake() + except SystemExit as e: + print("System exited prematurely") + print(e) + + assert app.foo == "bar" + class TestGenBoutiques: def test_boutiques_descriptor(self, tmp_path: Path, app: SnakeBidsApp):