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

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

Merged
merged 4 commits into from Apr 12, 2017

Conversation

@philippjfr
Copy link
Member

@philippjfr 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
Copy link
Contributor

@jlstevens 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
Copy link
Member Author

@philippjfr 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
Copy link
Contributor

@jlstevens jlstevens commented Apr 12, 2017

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

Makes sense.

@philippjfr philippjfr force-pushed the bokeh_apps branch from 7a61ed2 to 949435b Apr 12, 2017
@philippjfr
Copy link
Member Author

@philippjfr 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
Contributor

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

This comment has been minimized.

@philippjfr

philippjfr Apr 12, 2017
Author 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
Author Member

Is your suggestion this?

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

This comment has been minimized.

@jlstevens

jlstevens Apr 12, 2017
Contributor

I suppose that's fine.

This comment has been minimized.

@jlstevens

jlstevens Apr 12, 2017
Contributor

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

This comment has been minimized.

@jlstevens

jlstevens Apr 12, 2017
Contributor

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
Copy link
Contributor

@jlstevens 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
Copy link
Member Author

@philippjfr 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
Copy link
Contributor

@jlstevens jlstevens commented Apr 12, 2017

Looks good. Merging.

@jlstevens jlstevens merged commit 5a50e7f into master Apr 12, 2017
4 checks passed
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
@philippjfr
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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants