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

Logger reports subject, session, run even if not requested #804

Closed hoechenberger closed 6 months ago

hoechenberger commented 10 months ago

I'm seeing the following output during preprocessing/run_ica:

│11:20:48│ ⏳️ sub-005 run-08 Using PTP rejection thresholds: {'eeg': 0.0006}
│11:20:48│ ⏳️ sub-005 run-08 Calculating ICA solution.

As you can see, the messages are grouped under run-08.

The logs are generated here: https://github.com/mne-tools/mne-bids-pipeline/blob/4f5e4654f508982092432effbd85eaee2a72a12d/mne_bids_pipeline/steps/preprocessing/_06a_run_ica.py#L406-L418

No run parameter is passed to gen_log_kwargs()

But there's still an old run variable loitering around by the time we call gen_log_kwargs(), and it seems the function then pulls it from the stack and uses it: https://github.com/mne-tools/mne-bids-pipeline/blob/4f5e4654f508982092432effbd85eaee2a72a12d/mne_bids_pipeline/_logging.py#L113-L124

Only solution for me for now was to add del run before the call to gen_log_kwargs(); then I get the correct output:

│12:36:50│ ⏳️ sub-005 Using PTP rejection thresholds: {'eeg': 0.0006}
│12:36:50│ ⏳️ sub-005 Calculating ICA solution.
larsoner commented 10 months ago

This is by design, it's much more helpful than it is harmful. It fixed a ton of cases where we didn't include information we should have

hoechenberger commented 10 months ago

@larsoner Yep I can see that! So what should we do about the incorrect logging this causes in some places, like the one I pointed out above? Shall we add some "cleanup" lines that del subject, session, run if necessary?

larsoner commented 10 months ago

Yes just del run or use a different name like this_run, either way works

hoechenberger commented 10 months ago

Let's please keep this open as a reminder. There are still several places where this incorrect kind of logging is happening. I will look into this when I have time.