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 regression in 0.12.11 #7268

Closed
bryevdv opened this issue Dec 1, 2017 · 10 comments · Fixed by #7269
Closed

push_notebook regression in 0.12.11 #7268

bryevdv opened this issue Dec 1, 2017 · 10 comments · Fixed by #7269

Comments

@bryevdv
Copy link
Member

bryevdv commented Dec 1, 2017

The code at https://stackoverflow.com/questions/47583601/update-a-bokeh-span-with-an-interact-element-in-jupyter-notebook works with 0.12.10 but not 0.12.11

Showing anything at all in the notebook seems to generate this error:

screen shot 2017-12-01 at 10 13 36

The plot will still show, but things like notebook comms do not function.

cc @mattpap @philippjfr

@bryevdv
Copy link
Member Author

bryevdv commented Dec 1, 2017

I forgot to check examples/howto/notebook_comms manually before release.

@bryevdv
Copy link
Member Author

bryevdv commented Dec 1, 2017

@mattpap my guess is maybe #7203 since I can't think of anything else offhand in 0.12.11 that touched this area but I have not verified yet.

@bryevdv
Copy link
Member Author

bryevdv commented Dec 1, 2017

dev3 works and rc1 does not, so definitely think #7203 is the culprit.

@bryevdv
Copy link
Member Author

bryevdv commented Dec 1, 2017

For reference the diff for those tags is here 0.12.11dev3...0.12.11rc1

@bryevdv
Copy link
Member Author

bryevdv commented Dec 1, 2017

@mattpap this defer appears to be the problem:

0.12.11dev3...0.12.11rc1#diff-04078835a8fb40a2aae8c6f755938800R169

The comm message is coming in and being handled by jupyter before the comm is set up. Why is this defer needed?

The clearOutput error I originally reported is unrelated I think, it just needs some defensive code possibly to ignore things when there is nothing to clear.

@mattpap
Copy link
Contributor

mattpap commented Dec 1, 2017

defer() is there to detach expensive computations that follow from the initial processing (loading scripts, parsing HTML, etc.). This allows the browser to finish loading document before it will be stuck with expensive computations, and allows GC to catch a breath. This increases performance significantly and allows for better utilization of memory.

It's definitively easier to program stuff without it, but it must be there. As a temporary measure, you could use the non-deferred version in notebook. Though in the long term, notebook code must be able do deal with deferred code.

@bryevdv
Copy link
Member Author

bryevdv commented Dec 1, 2017

Though in the long term, notebook code must be able do deal with deferred code.

I not sure that is possible. We have no control over when the jupyter python side sends comms messages. AFAICT They are expected to be set up immediately and ready to go as soon as the cell publish message completes. Which means the documents associated with the comms have to be set up then as well.

@bryevdv
Copy link
Member Author

bryevdv commented Dec 1, 2017

OK, working on a PR to split the notebook embed codepath, there's too much optional arg checking due to partial case overlap anyway.

@parmentelat
Copy link

Hello there

I am running into this symptom a lot
I mean using push_notebook(handle) in a jupyter notebook, and getting

ValueError: PATCH-DOC message requires at least one event

I am using bokeh-0.12.13, I did try other variants in the 0.12.* family after I saw some hint that it could be a regression introduced between .10 and .11, but to no avail.

I guess my question is, is this fix present in 0.12.13 ? if so, what is the smallest version number that comes with this fix ? otherwise when is it expected to ship ? (I use pip3 against pypi)

thanks

@bryevdv
Copy link
Member Author

bryevdv commented Jan 16, 2018

@parmentelat That is not this issue. AFAICT that only happens if you call push_notebook and have not actually made any changes. In that case there are "no events", because there are no changes. It's possible we can do something better than we are currently doing (e.g. maybe just ignore when that happens instead of raising an exception) but that should go in a new separate issue.

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.

3 participants