Improved API for PREP configuration

[a-hurst]

Through the last few PRs, I've noticed there's quite a bit of configurable options through PyPREP that aren't actually exposed at the level of the PrepPipeline object (e.g. all of the NoisyChannels threshold parameters). In addition, the arguments/options that are currently exposed are divided between the params dict and actual keyword arguments in a mix of MatPREP's struct-based config style and a more Pythonic approach.

As an improved way of handling things, I was wondering if a PrepSettings object might be a good idea: instead of a difficult-to-document dict or a bunch of duplicated kwargs passed down multiple levels, PyPREP would have a properly-documented class that would initialize defaults, be able to validate parameter values, and expose methods and getters/setters for things to configure.

For example, if you wanted to run the Pipeline with a maximum of 3 robust reference iterations and a relaxed bad-by-deviation threshold of 3.29, on a file with a powerline frequency 50 Hz:

config = PrepSettings(line_freq=50)
config.reference_settings(max_iterations=3)
config.bad_by_deviation_settings(threshold=3.29)

pipeline = PrepPipeline(raw, config, montage)
pipeline.fit()

What do you think, would something like this make for a better API going forward?

[sappelhoff]

I like this idea 👍

Not sure whether something for 0.4 or later - I guess that'd be up to whoever wants to implement this :-)

Any opinions @yjmantilla ?

[a-hurst]

Not sure whether something for 0.4 or later - I guess that'd be up to whoever wants to implement this :-)

Any opinions @yjmantilla ?

I guess it depends on your comfort level with breaking API changes between now and 0.4.0 vs 0.4.0 and 0.5.0. Implementing the whole thing with all the options should definitely be a 0.5.0 thing, but doing the bare minimum to replace the params dict with this for 0.4.0 and remove some of the more unwieldy kwargs before a "stable" release might be less of a headache now than when PyPREP has officially left its "experimental" stage and has a larger userbase.

[yjmantilla]

My intuition is that the change would be for the better.

Implementing the whole thing with all the options should definitely be a 0.5.0 thing

I agree, there is a lot going on already hehe

but doing the bare minimum to replace the params dict with this for 0.4.0 and remove some of the more unwieldy kwargs before a "stable" release might be less of a headache now than when PyPREP has officially left its "experimental" stage and has a larger userbase

Yeap, definitely this is something before the stable (1.0.0 (?)) version. Which makes me thing that actually it is good to implement it now so as to actually see any problems with the implementation (and sorting those problems out) before pyprep has an official stable API.

[sappelhoff]

Okay, I am fine with either option: 1) implementing it fully for the upcoming 0.4 release, or 2) implementing it partially for 0.4, and fully for 0.5.

It'd probably be less disruptive to do 1), mainly because 0.4 has so many changes already, that a few additional ones wouldn't significantly increase the burden to update pipelines. And if we do a lot of changes in 0.4, then perhaps 0.5 can be a lot less disruptive.

[a-hurst]

@sappelhoff my idea with option 2 would be to reimplement all the parameters that PyPREP already exposes via PrepPipeline for 0.4.0 (e.g. line noise frequencies to remove, reference channels, max. reference iterations), and then expose additional options that aren't currently configurable from that level (e.g. all the NoisyChannels parameters) for 0.5.0.

That would fully establish the new API for 0.4.0 and get all of the potential workflow-breaking stuff out of the way; for 0.5.0 we'd just be extending the API with more features and options. Does that make sense?

[sappelhoff]

I agree that'd be the best way to go