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

add image_kwargs to report #859

Closed SophieHerbst closed 5 months ago

SophieHerbst commented 5 months ago

Responds to issue #848

Now that mne has been updated, we can pass image_kwargs to report.add_epochs. It works when I set the limits to the rejection limits in the config, but I cannot get it right when setting image_kwargs to None as per default.

 File "/home/sh254795/Documents/REPOS/mne-study-template/mne_bids_pipeline/_config_import.py", line 340, in _default_factory
    idx = allowlist.index(val)
ValueError: {'image_kwargs': None} is not in list

    assert val == typ(), (key, val)
AssertionError: ('report_add_epochs_kwargs', {'image_kwargs': None})
SophieHerbst commented 5 months ago

Not sure what to do about the tests. The doc consistency I don't understand and the ubuntu-latest seems to be using the latest stable mne where the changes in report.add_epochs are not yet included? Also, there might be further changes necessary with the reworking you did on the ICA?

SophieHerbst commented 5 months ago

Also, it turns out that in order to use the rejection criteria from my config, I had to deal with the different scaling between rejection and plotting commands:

# add image kwargs for report
report_add_epochs_image_kwargs = {
"grad": {"vmin": 0, "vmax": reject["grad"]*1e15},
"mag": {"vmin": 0, "vmax": reject["mag"]*1e15},
  }
larsoner commented 5 months ago

For the consistency we should merge https://github.com/mne-tools/mne-bids-pipeline/pull/863 first probably, it should fix things for you I think

We should maintain compat with latest MNE stable, so you'll need to inspect.getfullargspec(report.add_epochs) to determine whether or not to set image_kwargs I think

SophieHerbst commented 5 months ago

I think I need some help here

SophieHerbst commented 5 months ago

Thank you @larsoner I ran it again on my data and I am very happy with it now.

hoechenberger commented 5 months ago

Thanks @SophieHerbst and @larsoner!!!