Fix: Bind browser close output to creation context

[mh105]

Reference issue (if any)

None

What does this implement/fix?

In Jupyter notebook, closing a second or more Raw.plot() Qt browser appends its Channels marked as bad: report to the output of the first browser cell rather than the cell that created that browser. This behavior is a bit counterintuitive, since I expect to see the "Channels marked as bad:" message to appear in the same cell I invoke the function.

I asked Codex to look into the issue, and it found that IPykernel associates output with a ContextVar. A persistent Qt event-loop task will retain the context of the cell that initialized it, so a later close callback runs with an older output parent even though MNE’s logger writes to the current sys.stdout.

Codex proposed a minimal change by capturing contextvars.copy_context() independently when each MNE browser is created, then runs the existing shared close implementation in that captured context. The change looks good to me.

Additional information

Codex-generated tests passed:

• Focused browser-context unit tests passed (3 passed).
• Matplotlib browser selection-child close smoke test passed.
• A real IPykernel 7.3.0 plus Qt two-browser probe reproduced the stale parent before the fix and routed each close report to its own cell after the fix.

[larsoner]

I think we'll need a bit more info in the code for the next person to follow. Can you link to some ipython / jupyter docs that talk about this as a potential solution? A quick search turned up ipython/ipython#11565 but maybe there is something better.

Also, is this something we should special-case to the matplotlib inline backend? (Or maybe we don't need to because it's harmless on any others?)

[mh105]

Good question. I manually traced the chain further with the help of Codex to dig up relevant past PRs.

Background
mne-qt-browser PR #80 added the automatic %gui qt integration so that a browser created with block=False remains responsive in IPython. This establishes a persistent Qt event-loop callback in IPykernel, initially scheduled from the execution context of the cell that enables the loop.
IPykernel PR #1186 about two years later made output routing context-aware: IPykernel stores the Jupyter parent_header in a ContextVar so output generated by an asynchronous task is returned to the cell that scheduled that task. That is normally the desired behavior. In this case, however, the persistent GUI event loop services browsers created by multiple later cells while retaining the context—and therefore the output parent—of the first cell.

Our fix
IPython issue #11565 provides useful background on the underlying behavior. The discussion recommends explicitly retaining and reusing a Context when continuity is required. This is the same idea used in our fix here: copy_context() captures the context when each browser is created, and Context.run() restores it when that browser’s deferred close callback executes.

Some more digging
I also verified that this issue is not specific to the Matplotlib inline backend. The same incorrect placement of printed-message behavior occurs with both the mne-qt-browser backend and MNE’s Matplotlib browser using QtAgg. So I think it's better not to special-case to the inline backend. In fact, I believe the inline backend is non-interactive so it doesn't even trigger the _close() method call the same way these other interactive backends would in the first place. With the inline backend, the image is just displayed, and no interactive marking of bad channels with it is possible.

More generally, I was debating whether this should be a MNE-Python fix or a mne-qt-browser fix. I was convinced by Codex that it's better to do it here because the affected close handling in BrowserBase here impacts all backends, which include the matplotlib and qt supported by MNE-Python and the MNEQtBrowser implemented by mne-qt-browser.

One nice thing about this fix using self._close_context.run(self._close_impl, event) inside the _close() method is that copy_context() according to python documentation always returns a Context object, even if an empty Context, from which the .run() method works. So this fix is actually quite harmless and future proof on context related events we add in the future. I asked Codex to verify that self._close_context.run() works across all backends in MNE-Python (Agg, Inline, widget, QtAgg, mne-qt-browser) as well as a brand-new python thread with empty context.

Broader adoption?
One thing I was thinking when chasing through this rabbit hole is that we might want to broaden this fix to all other MNE-defined figure classes that have some kind of context-dependent event handling. This PR's fix will handle this for the 2D browser when closing the figure, but I think Epochs.plot() and ICA.plot_sources() still have their contexts tied to whatever earlier cells that might have started a persistent Qt event-loop callback in IPykernel.

If @larsoner you agree with my investigations here, then I think we may want to mirror this use of copy_context() to all other MNE defined figures. In fact, this should affect not only the _close() method, but whatever method in MNE figures that could trigger the context-aware stdout routing. This is obviously a bigger change, so I didn't want to crowd this PR for now. Let me know what you think!

[larsoner]

One thing I was thinking when chasing through this rabbit hole is that we might want to broaden this fix to all other MNE-defined figure classes that have some kind of context-dependent event handling.

I think we can tackle those when we hit them... the raw browser is the only one that immediately comes to mind as printing some "this is what the result was" message at the end, so it's at least the biggest case I think.

ipython/ipython#11565 provides useful background on the underlying behavior. The discussion recommends explicitly ...

Okay can you add a code comment linking to the specific comment that recommends this? It'll help us follow the trail on this issue next time someone is modifying that code or if we otherwise have problems with it

[mh105]

Sounds good. I linked to the PEP 567 asyncio section that describes this pair of copy_context() and Context.run(). I don't think others have explicitly described its use for persistent Qt event-loops yet, but our use case here fits the need for asynchronous callback and what ipython/ipython#11565 recommends to explicitly retain and reuse Context. Perhaps the persistent Qt event-loop callback creation could be made more specific, but that's auto-managed by IPykernel so it will be a much more involved fix than us explicitly using the figure-creation-context to close and print messages within MNE-Python.