-
Notifications
You must be signed in to change notification settings - Fork 946
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
feat: Thread safe kernel side Output widget #3759
base: main
Are you sure you want to change the base?
feat: Thread safe kernel side Output widget #3759
Conversation
Example code that works in the notebook: import threading
import time
import ipywidgets as widgets
def run1():
with out1:
for i in range(10):
display("output to 1")
time.sleep(0.2)
def run2():
with out2:
for i in range(10):
display("output to 2")
time.sleep(0.2)
out1 = widgets.Output()
out2 = widgets.Output()
threading.Thread(target=run1).start()
threading.Thread(target=run2).start()
widgets.HBox([out1, out2]) This would totally break before this PR, all output seems to go to 1 of the widgets. |
This hits on the fundamental decision we had to make when originally implementing output widgets and evaluating the experiments at ipython/ipykernel#115: Do we make the kernel more complicated (with an output redirection mechanism) and the frontend simple (which arguably is a better architecture), or do we make the frontend more complicated and need no changes to any kernel? We decided there were likely to be far more kernels than frontends, so the balance shifted to the somewhat awkward architecture we have today, which is kernel agnostic. In practice, though, there have been fewer kernels implementing ipywidgets support and more frontends implementing ipywidgets support than (at least I) anticipated. So I'm glad you are reevaluating the original tradeoff. |
A display hook can handle a publish message, but not yet a clear_output Upstreaming of: widgetti/solara#132 Follow up of: ipython#1110 Related: https://github.com/ipython/ipykernel/pull/115/files Would enable jupyter-widgets/ipywidgets#3759 to fully work kernel side.
Thank you for the context Jason! Note that this also requires ipython/ipykernel#1135 to be able to intercept the clear_output |
Background
Working on support the output widget in solara:
I found out about the hooks in ipykernel ipython/ipykernel#115
This approach will probably be better way forward.
The issue
Without a frontend, the output widget does not work. We had to work around that for Voila voila-dashboards/voila#91 (now ported to nbclient jupyter/nbclient#68 ).
Futhermore, the frontend is not aware of threads, causing all kinds of issues, e.g.:
Output.clear_output()
in a thread does not block until the output is cleared #3260This does require ipython/ipykernel#1110, so even if we go forward with this, the old code will have to stay in.
Testing and ipykernel
This solution only works with ipykernel/ipython, and I am not sure how to test this. Also, this goes against the direction of begin independent of a particular implementation. For instance, I am not sure xeus-kernel could support this (cc @martinRenou ).