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

push_notebook does not work in Google Colaboratory #9302

Closed
sirbiscuit opened this issue Oct 16, 2019 · 15 comments · Fixed by #10115
Closed

push_notebook does not work in Google Colaboratory #9302

sirbiscuit opened this issue Oct 16, 2019 · 15 comments · Fixed by #10115

Comments

@sirbiscuit
Copy link

Hello!

I tried to create an interactive plot with bokeh in Google Colab. The following code renders a scatter plot as expected:

from bokeh.models import ColumnDataSource
from bokeh.plotting import figure
from bokeh.io import output_notebook, push_notebook, show
import numpy as np

output_notebook(hide_banner=True)

source = ColumnDataSource(data=dict(x=np.random.rand(10), y=np.random.rand(10)))
p = figure(output_backend="webgl")
scatter = p.scatter(source=source, x='x', y='y')
handle = show(p, notebook_handle=True)

Changing the data and calling push_notebook should change the positions of the points in the scatter plot:

scatter.data_source.data['x'] = np.random.rand(10)
scatter.data_source.data['y'] = np.random.rand(10)
push_notebook(handle)

Instead, the notebook crashes ("Runtime disconnected"). This happens with bokeh version 1.3.4. You can find a demo of the problem here.

@mattpap
Copy link
Contributor

mattpap commented Oct 16, 2019

As far as I can tell, google colab doesn't support comms, nor in fact is a proper jupyter notebook environment. bokehjs warns in JS console about not being able to set up comms. It's unfortunate that colab crashes instead of giving a meaningful error message. I also notice that loading bokehjs' resources doesn't work very well (e.g. missing banner and gl bundle).

Unless comms are properly implemented, I doubt that we will be able to do anything about this, besides making the warning more prominent. However, I see that there is some support available for jupyter widgets. We are currently implementing jupyter wigets support in bokeh, so maybe this would allow to workaround those issues, but that's just a very remote maybe.

@bryevdv
Copy link
Member

bryevdv commented Oct 17, 2019

There is already an issue about Bokeh and comms on the colab issue tracker:

googlecolab/colabtools#217

I am going to close this as upstream as there is nothing at all we can do on our end until Google decides to whitelist or otherwise support our usage.

@blois
Copy link

blois commented Oct 18, 2019

I'm looking into this for Colaboratory-
For background Colab renders every cell's output in a separate iframe- this has been a longstanding requirement from Google's security team to sandbox the rendered outputs from credentials in the rest of the page. This is unfortunately not something we will be able to change.

Colab has the infrastructure for comms support but we have not exposed much of it. I'm curious how Bokeh would expect it to be exposed. Looking at

if (typeof Jupyter !== 'undefined' && Jupyter.notebook.kernel != null) {
Bokeh has support for Jupyter Notebooks and Jupyter Lab. Just to experiment I tried exposing Colab's comms via the Jupyter Notebook path- this causes a lot more problems because once we define window.Jupyter then other code starts assuming it's in a complete Notebook environment and lots of things break. For example this line causes the Bokeh initialization to completely break:
if (root.Jupyter !== undefined) {
.

@mattpap You mention that you are working on Jupyter Widgets support- unfortunately Colab currently only supports the core widgets. We would like a more extensible system and started a discussion a while back, but have not been able to make much progress. Some of the issues are covered in https://github.com/nteract/nes/tree/master/portable-widgets.

For the crash- is Bokeh using binary comms by chance? That could be hitting googlecolab/colabtools#587. We have not prioritized that issue as not too many users have hit it and there's a workaround for the only case we were seeing.

I am interested in improving Bokeh support in Colab- I addded some Bokeh initialization a while back (https://github.com/googlecolab/colabtools/blob/master/google/colab/_import_hooks/_bokeh.py).

I see a few approaches, and would appreciate feedback on which is preferred:

  • Expose a Jupyter-Notebook-like comms API google.colab.kernel.comm_manager and have Bokeh add explicit support for that.
  • Push the 'Portable Widgets' proposal harder.
  • Add a Widget-like API with less IPywidgets compatibility but more potential to be portable to other notebook environments.

@bryevdv
Copy link
Member

bryevdv commented Oct 18, 2019

@blois thanks for chiming in here. It would definitely be great the make something work, if possible. For this question I need a bit more context:

For the crash- is Bokeh using binary comms by chance?

The Bokeh protocol sends binary mode websocket messages for array buffers. But I am not sure it you are referring to things like that, at the general websocket level, or if this is about some Jupyter-speciic facility for binary comms. If the latter, the answer is no.

@blois
Copy link

blois commented Oct 18, 2019

@bryevdv - yes that would be hitting googlecolab/colabtools#587.

@bryevdv
Copy link
Member

bryevdv commented Oct 18, 2019

@blois Ok, thanks for the clarification, I did read the issue but it was not quite clear to me. But I also dug into things on our end and can confirm we are using the buffers argument:

bokeh/bokeh/io/notebook.py

Lines 270 to 275 in f1bc5b0

handle.comms.send(msg.header_json)
handle.comms.send(msg.metadata_json)
handle.comms.send(msg.content_json)
for header, payload in msg.buffers:
handle.comms.send(json.dumps(header))
handle.comms.send(buffers=[payload])

I had forgotten about that. FWIW That is pretty much the entire extent of our comms usage on the Python side.

@bryevdv
Copy link
Member

bryevdv commented Apr 7, 2020

@blois has here been any update in this area? Is there any mechanism to use binary comms?

@blois
Copy link

blois commented Apr 7, 2020

I am working on this on two fronts right now-

Once the binary comms plumbing is in Colab we should be able to either expose some globals in the environment or if the above jupyter-output-spec stabilizes and is acceptable then implement that.

@bryevdv
Copy link
Member

bryevdv commented Apr 7, 2020

Awesome thanks for the update, look forward to trying things out!

@blois
Copy link

blois commented May 14, 2020

The API is in, initially only exposed for users in an allow-list to limit impact of breaking changes. If you'd like to have it enabled feel free to email me blois at google com. Hopefully this is the last part for comms- making sure the APIs do what is needed.

Colab still has some work to do to upgrade tornado, which is a pre-req for upgrading Bokeh, but we're working on it.

Edit- example of the API is https://colab.research.google.com/gist/blois/e7abc90f2edcfab9a5c585cfda1c38ff/comms.ipynb

@philippjfr
Copy link
Contributor

philippjfr commented May 14, 2020

I'm working up a PR to add support for those comms to bokeh (and the HoloViz stack). I'll reopen this issue since the ball is now in our court.

@philippjfr philippjfr reopened this May 14, 2020
@philippjfr philippjfr added this to the 2.1 milestone May 14, 2020
@bryevdv
Copy link
Member

bryevdv commented Jun 3, 2020

@philippjfr any update on this? It would be nice to report this for 2.1

@blois
Copy link

blois commented Jun 3, 2020

Please let me know if there's anything I can do to help out here.

@philippjfr
Copy link
Contributor

Been swamped with a few new client projects and AnacondaCON the last two weeks. Let's discuss in the meeting today but I should have time over the weekend.

@faizankshaikh
Copy link

Hi - could you comment on which version this works for? I can't seem to get working on bokeh==2.1.1

Steps to recreate - https://gist.github.com/faizankshaikh/e23db80bdcdcb14176d492d505ee46b4

PS - I frankly think the issue I face is much simpler (and probably unrelated) to this issue. But still posting it here in the hope that resolving this issue solves the problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants