Fix routing of background thread output when no parent is set explicitly

[minrk]

• removes guessing about whether threads should route output to their originating cell (see Need public opt-out API for output routing from threads #1289)
• thread-local value is persisted, if set explicitly
• if no thread-local value is set, fallback on a global value (usually the latest cell), restores longstanding default behavior

There's lots of duplication around, but I couldn't see a way to resolve that without breaking things. That can be a later goal.

closes #1450

improves but does not quite close #1289 because there is still no explicit opt-out once a thread has set its parent (there is no clear_parent()), but at least the parent isn't set by default anymore, so the opt-out is less urgently needed.

To send a thread's output to a specific cell, this must now be done explicitly, which I think is a big improvement and simplification. No thread inheritance is tracked, no cell guessing occurs.

To route output to a specific cell from a thread, explicitly call set_parent in the thread with the parent from the outer context that should be kept for the duration of the thread:

current_cell_parent = get_ipython().get_parent()

def thread_task():
    get_ipython().set_parent(current_cell_parent)
    print("...")

Thread(target=thread_task).start()

[minrk]

passing everywhere but 3.14, includes missing coverage for #1450

[ianthomas23]

LGTM, just one typo.

[ianthomas23]

Typo mai to main ?

[minrk]

Good catch, thanks. Fixed.

[ianthomas23]

The failing qtconsole test

FAILED qtconsole/tests/test_00_console_widget.py::test_scroll[False] - pytestqt.exceptions.TimeoutError: waitUntil timed out in 20000 milliseconds

is labelled as flaky in qtconsole. The subshells changes in this repo made it more flaky, and it looks this PR might be making it more flaky still. I'd be inclined to merge this as it is, and if the flaky test in too annoying we could just exclude it from CI here until it can be investigated downstream. What do you think @minrk?

[minrk]

That sounds good to me

[ianthomas23]

Thanks @minrk

[ianthomas23]

I'm aiming for a 7.0.2 release soon with #1453, this PR, and hopefully a fix that I am working on for #1442/#1454.

[krassowski]

Just to understand, this made the behaviour from #1186 opt-in rather than opt-out?

[krassowski]

but at least the parent isn't set by default anymore, so the opt-out is less urgently needed.

Yeah, but it kind of brings back jupyter-widgets/ipywidgets#2358 and jupyterlab/jupyterlab#3936?

I understand the motivation for making it opt-out, and I understand that making it opt-in for now might be easier. If the plan is to release in 7.0.2 I would suggest highlighting this in the release notes, and documenting the workaround of using set_parent and get_parent in general.

[EwoutH]

Thanks for working on this. When will it be included in a new release?

[minrk]

@krassowski yes, though it is opt-in instead of unconditional, it never was 'opt-out' and since the guess was wrong very often, I think, having a clear opt-in is better than having no opt out at all, which is the choice we were presented with. Getting back to the default behavior once opted in is still a lot trickier than it should be, so room to improve there.

jupyter-widgets/ipywidgets#2358 should be addressed by Output.__enter__() setting the appropriate thread-local state. This is an explicit opt-in to capture output for the current thread, so it makes sense that it should do the capture. Note that #1186 also did not fix jupyter-widgets/ipywidgets#2358 either - it still routed output to the wrong place (cell output, not the widget OutputArea), but the same cell, rather than being captured by the OutputWidget.

I think having the thread-local state is still a great fix and the right thing to do, it's just the initial guess that was the problem. I think there are still improvements to be made on this front.

I 100% agree on documenting the change, I'll look into that.

[ianthomas23]

I've prepared a draft release of 7.1.0 as I realised that #1435 is an enhancement and hence needs to be in a minor not patch release. The draft changelog notes are at https://github.com/ipython/ipykernel/releases; I've written a little summary at the top for the changes but I have not covered this PR. @minrk Would you like to add a line or two of documentation there, or a link to wherever else you're planning to write it?

[DonJayamanne]

@ianthomas23 @krassowski
(apologies if I've pinged the wrong people)
Hi, I'm one of the maintainers of Jupyter extension in VS Code here.

Any idea when a new version of IPyKernel will be shipped out with this fix?
Asking as we have quite few a users running into issues due to this.

Thanks for promptly fixing this issue

[minrk]

@ianthomas23 thanks, I added a little note to the 7.1 release notes draft. Good to go from my end once I skip the most failing tests in #1463

[ianthomas23]

@ianthomas23 @krassowski (apologies if I've pinged the wrong people) Hi, I'm one of the maintainers of Jupyter extension in VS Code here.

Any idea when a new version of IPyKernel will be shipped out with this fix? Asking as we have quite few a users running into issues due to this.

Thanks for promptly fixing this issue

@DonJayamanne ipykernel 7.1.0 has been released.

[minrk]

@ianthomas23 thanks for shipping this!