Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
hoechenberger opened this issue Oct 31, 2023 · 4 comments · Fixed by #865
Closed

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

hoechenberger opened this issue Oct 31, 2023 · 4 comments · Fixed by #865
Labels
bug Something isn't working logging

Comments

@hoechenberger
Copy link
Member

hoechenberger commented Oct 31, 2023

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:

msg = f"Using PTP rejection thresholds: {ica_reject}"
logger.info(**gen_log_kwargs(message=msg))
epochs.drop_bad(reject=ica_reject)
if epochs_eog is not None:
epochs_eog.drop_bad(reject=ica_reject)
if epochs_ecg is not None:
epochs_ecg.drop_bad(reject=ica_reject)
# Now actually perform ICA.
msg = "Calculating ICA solution."
logger.info(**gen_log_kwargs(message=msg))
ica = fit_ica(cfg=cfg, epochs=epochs, subject=subject, session=session)

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:

stack = inspect.stack()
up_locals = stack[1].frame.f_locals
if subject is None:
subject = up_locals.get("subject", None)
if session is None:
session = up_locals.get("session", None)
if run is None:
run = up_locals.get("run", None)
if run is None:
task = task or up_locals.get("task", None)
if task in ("noise", "rest"):
run = task

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.
@hoechenberger hoechenberger added bug Something isn't working logging labels Oct 31, 2023
@larsoner
Copy link
Member

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
Copy link
Member Author

hoechenberger commented Oct 31, 2023

@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
Copy link
Member

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

@larsoner larsoner closed this as not planned Won't fix, can't repro, duplicate, stale Nov 7, 2023
@hoechenberger
Copy link
Member Author

hoechenberger commented Nov 7, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working logging
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants