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

@philippjfr philippjfr commented Aug 15, 2016

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

@jlstevens jlstevens Aug 15, 2016

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

@philippjfr philippjfr Aug 15, 2016

Will do.

@jlstevens
Copy link
Contributor

@jlstevens jlstevens commented Aug 15, 2016

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

@philippjfr philippjfr commented Aug 15, 2016

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

@jlstevens jlstevens commented Aug 15, 2016

Looks good! Merging.

@jlstevens jlstevens merged commit be9c6f4 into master Aug 15, 2016
2 of 3 checks passed
@jbednar jbednar deleted the bokeh_json_opt branch Aug 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants