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
134 stars 65 forks source link

DOC: Automatic flowchart #860

Closed larsoner closed 5 months ago

larsoner commented 5 months ago

Before merging …

Locally produces:

Screenshot from 2024-02-23 14-52-05

@hoechenberger feel free to push directly wording changes you want. Locally you can iterate pretty easily with:

~/python/mne-bids-pipeline/docs/site$ python -m http.server
~/python/mne-bids-pipeline/docs/site$ ../build_docs.sh

then refreshting http://localhost:8000/features/overview.html in your browser.

larsoner commented 5 months ago

@SophieHerbst can you see if this fixes the wording issue(s) you ran into or if there is more to do?

SophieHerbst commented 5 months ago

@larsoner is the 'temporal regression for artifact removal' a default step? I am not aware that I am using it. If not, I think it should be marked as optional.

hoechenberger commented 5 months ago

most of the steps are optional though.... we'd end up with almost everything being marked as such

SophieHerbst commented 5 months ago

mh right. I am wondering how to make it clearer which config options trigger which steps (or their re-run)

hoechenberger commented 5 months ago

there are get_config() functions in every step now which cover almost all configuration options that affect this step (exceptions are I believe n_jobs, subjects and session, but i'm not 100% certain from the top of my head)

in theory this information could be used

SophieHerbst commented 5 months ago

Used as in informing the documentation? (I learned about this part last week, when I forgot to mention the image_kwargs for the report there; easy to make mistakes as it needs to be added manually)

hoechenberger commented 5 months ago

Used as in informing the documentation?

Yes

We'd basically have to iterate over all steps and grep the arguments used in get_config() and then put that in a table or so…

larsoner commented 5 months ago

Opened #862 and #861 so we can iterate further in follow-up PRs. This one in the meantime is 90% a bugfix for missing / incorrect information at the moment (the other 10% being the ICA/SSP enhancement to try and make that part clearer) -- @SophieHerbst @hoechenberger happy with those parts so we can merge this?

larsoner commented 5 months ago

https://output.circle-artifacts.com/output/job/d69d1de1-f4fc-4714-9d6e-f43e8f3816b7/artifacts/0/site/features/overview.html

hoechenberger commented 5 months ago

yes, this is great! feel free to merge whenever you feel it's ready 👍👍👍