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

Improved bokeh json patch computation #807

Merged
merged 2 commits into from
Aug 15, 2016
Merged

Improved bokeh json patch computation #807

merged 2 commits into from
Aug 15, 2016

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Aug 3, 2016

Improved code that tidies up bokeh json patches to update plots. Should result in an overall reduction of 30-50% in the size of the json containing the plot updates.

Things to do:

  • Test on a wide range of plots to ensure nothing breaks

I've now tested this on every bokeh based notebook I could find and have not encountered any issues. Very happy I could condense the plot updates this much!

Improved code that tidies up bokeh json patches to update plots.
Should result in an overall reduction of 30-50% in the json size
@philippjfr
Copy link
Member Author

This is ready to be merged now, I've been running off this branch for almost two weeks now without any issues and it really does represent a huge saving in the amount of data that is sent to the browser.

"""
Returns a list of all ids in the supplied object.
Useful for determining if a json representation contains
references to other objects.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth mentioning why this knowing if the JSON has references is useful information (if it can be summarized in a simple way).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

@jlstevens
Copy link
Contributor

Ok, hopefully two weeks worth of testing means it is robust!

I am happy to merge once you've decided to either ignore/implement the two suggestions I've made in the comments.

@philippjfr
Copy link
Member Author

Just implemented your suggestions and improved various docstrings to make it a bit clearer how this stuff actually works. Ready to merge now.

@jlstevens
Copy link
Contributor

Looks good! Merging.

@jlstevens jlstevens merged commit be9c6f4 into master Aug 15, 2016
@jbednar jbednar deleted the bokeh_json_opt branch August 15, 2016 12:46
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants