search

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

BUG: Fix several bugs #839

Closed larsoner closed 6 months ago

larsoner commented 6 months ago

Before merging …

I went to work on #831 but wanted to sort out a few little other bugs (and one minor enhancement) first:

  1. Fix bug where using mf_reference_run = None could change the empty room file matched from run-to-run of mne_bids_pipeline due to use of set, whose order is not guaranteed. @hoechenberger I suspect this could be the actual root cause of #814

  2. Output the reject params in the reject step -- though this can be set in the config, it's sometimes calculated on-the-fly by autoreject and not saved anywhere. Even in the manual dict case it's nice to have it near the drop logs in the report in case you want to think about adjusting your thresholds:

    Screenshot from 2024-01-30 12-00-44

  3. Fix bug where reject was getting restricted incorrectly for M/EEG channels

  4. Prefer stat to lstat because I'm pretty sure we don't want to use the symlink's own mtime but rather the mtime of whatever it points to. (This is apparently the only difference between lstat and stat.) Doubt anyone has actually hit this so no changelog needed, but better to have it in there I think.

  5. Fix bug with -regress saving where instead of writing one for each run, a single filename was written to by every run (within-release change so no changelog needed).

  6. Slightly better report tags for the cleaned epochs to make it clear that what's happened is the reject step

hoechenberger commented 6 months ago

Thanks @larsoner!

I think the value of the "reject" parameter should be included via upstream changes to mne.Report.add_epochs(). But we can merge here and move the feature to MNE-Python at a later time. Would be good to create an issue as a reminder

larsoner commented 6 months ago

Found another bug! --no-cache didn't work. Pushing now. Also nesting the rejection threshold stuff under the Epochs: after cleaning section of the report. Marking for merge-when-green but let me know if you'd like me to hold off @hoechenberger !