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

ENH: Doc config in sheet #869

Closed larsoner closed 4 months ago

larsoner commented 4 months ago

Before merging …

Closes #868

Result:

@hoechenberger in principle we could split this into multiple rows, but I think it's perhaps nicer this way -- if you want to see a multiline value you click in the cell and you can see it, select, copy-paste etc. If we put in multiple rows it might not be as easy to do that.

image

The truncation code is still present, but it really only applies to the DigMontage because its json_tricks dump is so long (contains all channel locations, coord frame, etc from DigMontage.).

Also this contains all config values, both those supplied by the user and those that are defaults. I think it's nice because it shows all values that are used. If people want to know which values they supplied, that complementary information is in the _report.html files.

larsoner commented 4 months ago

And here's how I'm getting it to deal with callables by adding the path + function name:

image

larsoner commented 4 months ago

... @hoechenberger should we cut 1.6 after this PR?

drammock commented 4 months ago

... @hoechenberger should we cut 1.6 after this PR?

I know you're not asking me, but I would appreciate a new release, as I'm about to start in on a new project in the coming days (thus freezing the environment; would be nice to be on stable and have all the great additions you two have done lately)

hoechenberger commented 4 months ago

yes let's cut one, i'm generally a big friend of frequent releases

hoechenberger commented 4 months ago

@larsoner We depend on MNE 1.7 now, though

we should mention this in the changelog

not sure how to handle this properly, since 1.7 isn't out yet

we should run CI jobs that test against MNE stable

larsoner commented 4 months ago

@larsoner We depend on MNE 1.7 now, though

For which features? I made the add_epochs_image_kwargs optional. Right now it will silently ignore them if you aren't on 1.7+ but I could make it raise an error or emit a warning instead.

Was there something else that required 1.7+?

hoechenberger commented 4 months ago

@larsoner We depend on MNE 1.7 now, though

For which features? I made the add_epochs_image_kwargs optional. Right now it will silently ignore them if you aren't on 1.7+ but I could make it raise an error or emit a warning instead.

Was there something else that required 1.7+?

ah ok! this was the only feature I had in mind. We should at least document in the changelog that this feature will only work with MNE dev or 1.7 once released

larsoner commented 4 months ago

Okay pushed a commit with the doc requirement, good to merge this one?

hoechenberger commented 4 months ago

good to go for me!

larsoner commented 4 months ago

@hoechenberger okay for me to cut a basic release on GH and you can edit the notes afterward when you get a chance?

hoechenberger commented 4 months ago

yes let's do it 👍