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

Small Bokeh tutorial/code improvements #477

Merged
merged 9 commits into from Feb 8, 2016

Conversation

Projects
None yet
2 participants
@philippjfr
Copy link
Contributor

philippjfr commented Feb 8, 2016

Minor fixes for the Bokeh tutorials removing additional Element examples, which do not yet work, i.e. sidebar of ErrorBars and overlay of VectorFields. Also added bootstrap based warning messages to warn that Tutorials can take a minute to render because there's so many plots on the page.

"<div class=\"alert alert-info\" role=\"alert\">\n",
" This tutorial contains a lot of bokeh plots, which may take a little while to load and render.\n",
"</div>\n",
"\n",

This comment has been minimized.

@jlstevens

jlstevens Feb 8, 2016

Contributor

This does help but I was also thinking of submitting an issue to suggest some sort of loading placeholder for Bokeh plots. Would this be feasible/worthwhile?

This comment has been minimized.

@philippjfr

philippjfr Feb 8, 2016

Author Contributor

Does sound nice but I also don't know how feasible it is.

options.ErrorBars = Options('style', color='black')
options.Spread = Options('style', fill_color=Cycle(), fill_alpha=0.6, line_color='black')
options.Histogram = Options('style', fill_color="#036564", line_color="#033649")
options.Points = Options('style', color=Cycle())
options.Points = Options('style', color=Cycle(), size=np.sqrt(6))

This comment has been minimized.

@jlstevens

jlstevens Feb 8, 2016

Contributor

np.sqrt(6)? I assume you are trying to match something but it might be worth having a comment stating what exactly.

This comment has been minimized.

@philippjfr

philippjfr Feb 8, 2016

Author Contributor

True, will add it. Point sizes in bokeh are widths while matplotlib uses area and has a default of 6.

@@ -538,17 +538,44 @@ def __init__(self, element, plot=None, **params):

def initialize_plot(self, ranges=None, plot=None, plots=None):
self.mplplot.initialize_plot(ranges)
plot = mpl.to_bokeh(self.mplplot.state)

This comment has been minimized.

@jlstevens

jlstevens Feb 8, 2016

Contributor

How many BokehMPLWrapper plots are there? These plots depend on both matplotlib and bokeh. This isn't typically a problem but I would like to know how many of these there are.

This comment has been minimized.

@philippjfr

philippjfr Feb 8, 2016

Author Contributor

This is a general purpose wrapper class so there's only this one and the one that converts matplotlib plots to RGB images (which we should probably remove). Currently we only register the seaborn Element types for actual conversion and have the matplotlib 3D plots registered to use the RGB conversion. I'll actually go ahead and delete these:

Surface: PlotSelector(lambda x: 'bokeh',
                      [('mpl', SurfacePlot),
                       ('bokeh', BokehMPLRawWrapper)], True),
Scatter3D: PlotSelector(lambda x: 'bokeh',
                        [('mpl', Scatter3DPlot),
                         ('bokeh', BokehMPLRawWrapper)], True),
Trisurface: PlotSelector(lambda x: 'bokeh',
                         [('mpl', TrisurfacePlot),
                          ('bokeh', BokehMPLRawWrapper)], True)

as part of this PR.

This comment has been minimized.

@jlstevens

jlstevens Feb 8, 2016

Contributor

Ok great.

There probably isn't much value in having Bokeh wrap matplotlib as you will always be limited by the speed of rendering on the matplotlib side. In general nesting backends probably doesn't make sense and it is probably best to simply state that a particular element isn't supported by a given backend.

@@ -177,3 +179,33 @@ def hsv_to_rgb(hsv):
rgb = clist[order[i], np.arange(np.prod(shape))[:,None]]

return rgb.reshape(shape+(3,))


def update_plot(old, new):

This comment has been minimized.

@jlstevens

jlstevens Feb 8, 2016

Contributor

Seems like something that would ideally be offered by Bokeh itself (if I understand it right).

This comment has been minimized.

@philippjfr

philippjfr Feb 8, 2016

Author Contributor

Agreed, they're working on it. Will make a note that it should be replaced after bokeh 0.12.

@jlstevens

This comment has been minimized.

Copy link
Contributor

jlstevens commented Feb 8, 2016

I've made a few comments but otherwise it looks ok to me. That said, there are a number of code changes that I assume will affect more than just the Bokeh tutorial.

@philippjfr philippjfr changed the title Small Bokeh tutorial improvements Small Bokeh tutorial/code improvements Feb 8, 2016

@philippjfr philippjfr force-pushed the bokeh_tutorials branch from 07d5dd6 to 555f0d1 Feb 8, 2016

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

philippjfr commented Feb 8, 2016

Ready to merge now.

@jlstevens

This comment has been minimized.

Copy link
Contributor

jlstevens commented Feb 8, 2016

Ok. Tests are passing so I'll merge now.

jlstevens added a commit that referenced this pull request Feb 8, 2016

Merge pull request #477 from ioam/bokeh_tutorials
Small Bokeh tutorial/code improvements

@jlstevens jlstevens merged commit 40d3fd5 into master Feb 8, 2016

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.08%) to 69.477%
Details
s3-reference-data-cache Test data is cached.
Details

@jlstevens jlstevens deleted the bokeh_tutorials branch Feb 9, 2016

@philippjfr philippjfr added this to the 1.4.3 milestone Feb 10, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.