Basic Plugin Architecture

[pvandyken]

This is one of the practical manifestations of #255.

First, a basic overview of how SnakeBidsApps work:

SnakeBidsApp()                               .run_snakemake()     ??.add_plugins()
|                                            |                    |
|                                            |                    |
|-> Parse App Directory -> Create CLI parser |-> Parse arguments  |-> Run snakemake
    - locate Snakefile	   - Start with basic    - Update config
      and config             basic parser and      with the parsed
    - load config into       add items from        cli
      dict                   config          ^                     ^
                                             |                     |
			    			    Intervention Point 1      Intervention point 2

Intervention Point 1 - Available
Intervention Point 2 - Unavailable

The .run_snakemake() method gives access to intervention point 1, but point 2 is not currently accessible. There are many potential tools that need access to point 2, however, because it's only there that we get access to the parsed CLI arguments loaded into config.

Note that point 1 is still necessary for boutiques, which needs the constructed but unparsed CLI parser.

I propose the SnakeBidsApp.add_plugins() method, according to the following signature:

def SnakeBidsApp.add_plugins(
	self,
	plugins: list[Callable[[SnakeBidsApp], SnakeBidsApp | None]],
) -> SnakeBidsApp:
	"""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 can return a new or
	modified copy of the ``SnakeBidsApp`` or ``None``. 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.
	"""

Plugins provided via this method would be called after cli parsing and config updating, just before snakemake is invoked.

This architecture would be of immediate benefit to #259 and #256.

[tkkuehn]

Spent some time looking at a basic implementation of this today -- for now I'll just note that I don't think it makes sense for plugins as defined here to return a modified SnakeBidsApp. To use the returned SnakeBidsApp in the current framework, I think we would have to reassign self within the context of run_snakemake (briefly illustrated below), which I think might work but feels wonky. Maybe I'm not looking at this the right way though.

class SnakeBidsApp:
    ...
    run_snakemake(self):
        ...
        for plugin in self.plugins:
            self = plugin(self) or self  # I don't think this makes a ton of sense when SnakeBidsApp is mutable anyway

[pvandyken]

I think we would have to reassign self within the context of run_snakemake (briefly illustrated below), which I think might work but feels wonky.

In most cases I agree that SnakeBidsApp will just be mutated, but I didn't want to make that a strong assumption. If you don't want to reassign self (I agree that's weird), could you just make a new app variable that initializes as app = self and then gets overridden as per your example. And that gets used in the final snakemake call?

[tkkuehn]

Could you just make a new app variable that initializes as app = self and then gets overridden as per your example. And that gets used in the final snakemake call?

Good idea, I'm okay with this in principle. I do struggle to think of a real example where a plugin replacing the SnakeBidsApp provides something that mutating the original one wouldn't, but I don't think that should necessarily stop us from allowing it.

[tkkuehn]

I guess we do need to make clear in this case though that a SnakeBidsApp provided by a plugin must handle its own argument parsing, config updating, workflow mode handling, etc. (i.e. we will not redo the processing in run_snakemake that comes before running plugins), so it is kind of a "soft" requirement that the new SnakeBidsApp is derived somehow from the original.

[pvandyken]

I see, yeah if someone really starts from scratch they would have to redo parsing etc. More likely they're using copy.deepcopy() for whatever reason.

I can't think of any actual use cases right now, but it's trivial to implement, so I don't see a loss