mne-tools / mne-bids-pipeline

Automatically process entire electrophysiological datasets using MNE-Python.
https://mne.tools/mne-bids-pipeline/
BSD 3-Clause "New" or "Revised" License
133 stars 65 forks source link

Improve documentation and config validation of `loose` and `depth` parameters; drop support for `loose=None` #915

Closed hoechenberger closed 3 months ago

hoechenberger commented 3 months ago

@larsoner @SophieHerbst WDYT?

We currently don't support volumetric or mixed source spaces anyway, so I thought we could simplify things a little.

I was even considering entirely removing the loose and depth settings but then wasn't sure if some of you might be using them sometimes? Personally I never touch those.

Before merging …

larsoner commented 3 months ago

Eventually we could pretty easily add support for volumetric source estimates I think. So I'm not sure it's worth getting rid of 'auto' just to potentially add it back. And I think loose and depth are both useful from time to time, especially when working with template / non-individualized MRIs (you often want loose=1.)

SophieHerbst commented 3 months ago

I agree about keeping the loose and depth parameters, we do use them sometimes. No opinion about the 'auto' option, but if mixed source spaces can be implemented easily, that would be useful.

hoechenberger commented 3 months ago

@larsoner @SophieHerbst I now changed things to basically only improve the documentation and config validation, except for one change: I removed support for loose=None, as this is equivalent to looe=0, and I always disliked we had two ways to achieve the same outcome.

This, of course, introduces an inconsistency with MNE, which allows for loose=None, but I can live with that…

Thoughts?

SophieHerbst commented 3 months ago

I have always found the options for those parameters a bit confusing, so I am fine with removing the None.

larsoner commented 3 months ago

Okay with me to remove None

larsoner commented 3 months ago

Whoops wrong PR comment, thanks @hoechenberger !