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

Added BokehRenderer.app method to create bokeh apps in scripts and notebooks #1283

Merged
merged 4 commits into from Apr 12, 2017

Conversation

Projects
None yet
2 participants
@philippjfr
Member

philippjfr commented Apr 12, 2017

Adds convenience method to create bokeh Apps from any holoviews object. In a script it simply returns the doc but when used with notebook option creates an Application instance which is displayed and returned.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Apr 12, 2017

Great!

All it needs is an example script in plotting/bokeh/examples/apps/apps and maybe a script and notebook in holoviews-contrib.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Apr 12, 2017

All it needs is an example script in plotting/bokeh/examples/apps/apps

I'll just change the existing examples to use this.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Apr 12, 2017

I'll just change the existing examples to use this.

Makes sense.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Apr 12, 2017

Updated the examples, ready to merge once tests pass.

@@ -33,6 +33,9 @@
Store.renderers['bokeh'] = BokehRenderer.instance()
if len(Store.renderers) == 1:

This comment has been minimized.

@jlstevens

jlstevens Apr 12, 2017

Member

Why not check that the one available renderer is 'bokeh'?

This comment has been minimized.

@philippjfr

philippjfr Apr 12, 2017

Member

Guess I could but defining that renderer is literally the line above so it has to be bokeh.

This comment has been minimized.

@philippjfr

philippjfr Apr 12, 2017

Member

Is your suggestion this?

if list(Store.renderers.keys()) == ['bokeh']

This comment has been minimized.

@jlstevens

jlstevens Apr 12, 2017

Member

I suppose that's fine.

This comment has been minimized.

@jlstevens

jlstevens Apr 12, 2017

Member

You last post appeared before my reply. Either way is fine.

This comment has been minimized.

@jlstevens

jlstevens Apr 12, 2017

Member

In that case, I would mention that in a script-level docstring at the top of the file. Same for the crossfilter app. It only needs to be one line explaining what these examples are supposed to show.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Apr 12, 2017

Thanks for fixing the examples/apps/app nesting. I notice player.py was renamed but not changed..was that your intention?

@philippjfr

This comment has been minimized.

Member

philippjfr commented Apr 12, 2017

Yes, player does not use BokehRenderer.app it's an example of creating a custom bokeh layout with custom widgets.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Apr 12, 2017

Looks good. Merging.

@jlstevens jlstevens merged commit 5a50e7f into master Apr 12, 2017

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.2%) to 78.825%
Details
s3-reference-data-cache Test data is cached.
Details

@philippjfr philippjfr deleted the bokeh_apps branch Apr 19, 2017

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