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

node.js integration and static image generation #3047

Closed
wants to merge 31 commits into from

Conversation

mattpap
Copy link
Contributor

@mattpap mattpap commented Nov 2, 2015

At this point you can convert a file_html() generated HTML file into a set of PNG images (one per canvas). For example:

cd bokeh/bokehjs
(cd ../examples/plotting/file; python bollinger.py)
node render.js ../examples/plotting/file/bollinger.html
eog ../examples/plotting/file/bollinger-PLOTID.png

Currently you need to install these manually:

sudo apt-get install libcairo2-dev libpango1.0-dev libjpeg-dev libgif-dev

Issues:

  • fonts are messed up very often (when using custom font/size/face)
  • several examples are simply broken (histogram, burtin, gears, etc.)
  • examples involving maps don't terminate (disabling maps helps)
  • examples involving ImageURL, maps and ??? are unreliable (see render:done)
  • examples/plotting/file/image.py (RangeError: Source is too large)
  • examples/plotting/file/image_rgba.py (TypeError: Image or Canvas expected)

TODO:

  • add direct support for image generation from bokeh
  • use separate package.json for node.js integration tools
  • add support for custom models
  • make render:done event reliable

@mattpap mattpap self-assigned this Nov 2, 2015
@mattpap mattpap added this to the 0.11.0 milestone Nov 2, 2015
@@ -117,7 +117,19 @@ fill_render_item_from_script_tag = (script, item) ->

logger.info("Will inject Bokeh script tag with params #{JSON.stringify(item)}")

embed_items = (docs_json, render_items, websocket_url) ->
embed_items = (docs_json_or_id, render_items, websocket_url) ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nicer perhaps if we either always do it the "embed the doc as a script tag" way or the "embed the doc as JSON" way, rather than sometimes doing one and sometimes doing the other.

It looks like you added the "embed doc as a script" way in order to scrape a document out of an HTML file?

If so I would say we should always go from Application to PNG/SVG, rather than scrape-json-from-html-file to PNG/SVG. The Application could be spelled as a JSON file, which gives you the JSON case (add application/spellings/json.py which does Document.from_json_string and you have that).

Command line implications:

  • bokeh json myapp.py - dumps myapp.py as JSON (creates app document and saves document.to_json_string())
  • bokeh html myapp.json - sticks the JSON in an HTML file (needs a json spelling handler)
  • bokeh png myapp.py, bokeh png myapp.json - output the app from whatever format to PNG

I know you aren't done with the PR so maybe you've already gone in another direction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like you added the "embed doc as a script" way in order to scrape a document out of an HTML file?

This is a secondary goal (thought useful for testing), but really I wanted to externalize the JSON for quite some time already. This is not the final form, because of render_items and custom models.

(...) rather than scrape-json-from-html-file (...)

Obviously, but as I said, the current approach is good for testing. I'm currently working on the proper API/CLI.

Copy link
Contributor

Choose a reason for hiding this comment

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

if we end up with both externalized docs_json and in-script docs_json, let's make two separate functions here in Bokeh.embed rather than type-introspecting the parameter...

@havocp
Copy link
Contributor

havocp commented Nov 18, 2015

It looks like the version of node we have in travis is wrong? travis is having a meltdown on this PR

custom_models = self._collect_custom_models()
if custom_models:
json['models'] = custom_models

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will cause us problems; models code is not part of Document. Here we are tacking an extra payload on to Document that we'd like to go to JavaScript, because we happen to know we'll send the Document to JavaScript.
It's an unexpected side effect that Document.from_json on the coffee side loads custom models, it should only fill in Document, not have surprising side effects.

What I think would be cleaner: in _standalone_docs_json_and_render_items, redefine docs_json to include the models (maybe demote the current dict of docs, so it's { 'models' : { ... }, 'docs' : current_docs_json_here } and then in Bokeh.embed.embed_items of course the code has to be changed to match.

Copy link
Contributor

Choose a reason for hiding this comment

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

another advantage of doing it in _standalone_docs_json_and_render_items is that if there's multiple docs we only send the models once

raise RuntimeError("Unable to save object of type '%s'" % type(obj))

if validate:
doc.validate()
Copy link
Contributor

Choose a reason for hiding this comment

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

the code from with _ModelInDocument through validate() could probably be factored out since it's in _save_helper as well

@damianavila
Copy link
Contributor

@mattpap what is the status on this one?

Conflicts:
	bokehjs/src/coffee/renderer/glyph/image_url.coffee
@mattpap
Copy link
Contributor Author

mattpap commented Nov 27, 2015

@damianavila, this needs some more work. I'm not sure if this will be ready by 0.11 deadline.

@bryevdv
Copy link
Member

bryevdv commented Nov 27, 2015

@mattpap @damianavila we should discuss as a team. I see 0.11 as a feature-driven release, so if we need to slip the schedule I am not opposed, and we can see what the whole team thinks. If we can even just lay the low-level groundwork in 0.11 then additional incremental improvements would make for great 0.11.x releases. My main concern is getting through as much low-level churn in 0.11 as possible, so that there are only smaller or incremental changes afterwards.

@damianavila
Copy link
Contributor

@mattpap @damianavila we should discuss as a team. I see 0.11 as a feature-driven release, so if we need to slip the schedule I am not opposed, and we can see what the whole team thinks. If we can even just lay the low-level groundwork in 0.11 then additional incremental improvements would make for great 0.11.x releases. My main concern is getting through as much low-level churn in 0.11 as possible, so that there are only smaller or incremental changes afterwards.

I agree with this view... let's talk about this in the meeting... ping @bokeh/core

@mattpap mattpap modified the milestones: 0.11.1, 0.11.0 Jan 4, 2016
@bryevdv
Copy link
Member

bryevdv commented Jan 19, 2016

@mattpap this version of the PR can be closed?

@mattpap
Copy link
Contributor Author

mattpap commented Jan 19, 2016

@bryevdv, yes. I was just waiting with this, so I could supersede this PR in one step. Can be done in two.

@bryevdv
Copy link
Member

bryevdv commented Jan 19, 2016

superseded by future work

Left the branch intact for now.

@bryevdv bryevdv closed this Jan 19, 2016
@damianavila damianavila removed this from the 0.11.1 milestone Jan 20, 2016
@mattpap mattpap deleted the mattpap/1589_nodejs branch February 12, 2018 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants