-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
Add support for HTML export after moving to MIME bundle #2574
Conversation
Isn't this arguably a bug in nbconvert? Shouldn't the output of |
No.
No, the output of |
That doesn't work when you have to separate HTML from JS which you now have to do for Jupyterlab. I agree this is by 'design' but that design now looks broken to me. |
holoviews/plotting/renderer.py
Outdated
element_id = plot_id | ||
html = "<div id='%s' style='display: table; margin: 0 auto;'>%s</div>" % (element_id, html) | ||
if not os.environ.get('HV_DOC_HTML', False) and js is not None: | ||
js = embed_js.format(element_id=element_id, plot_id=plot_id, html=html) + js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why define element_id
if you can just replace this with plot.id
(or plot_id
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, I see it now. Though isn't it plot.id
in both branches?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bit confusing but in the widget case they aren't identical, in that case plot
variable is actually the widget and the two are different. I might just call it widget_id
instead of element_id
and then default it to None in the non-widget case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. If it can be made clearer, then great!
@@ -69,6 +69,17 @@ | |||
</html> | |||
""" | |||
|
|||
embed_js = """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest leaving a comment pointing to this PR where we can explain why such a nasty thing exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, don't want anyone blaming us for this.
Looks as good as it is going to get and the tests have finally gone green. For anyone directed to this PR from the JS comment, this is a very ugly hack that we did not want to include in our code. Unfortunately, the alternatives were even less palatable 1) prematurely drop support for classic Jupyter notebook in favor of JupyterLab 2) send all the data to the notebook twice. Given that we found those alternatives unacceptable, we opted for this approach which should only ever execute in the context of HTML exported using nbconvert. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
When we switched to rendering our plots using the
_repr_mimebundle
we lost all support for exporting bokeh plots and holoviews widgets to standalone HTML files. This is because nbconvert will only render a single MIME type, whichever is highest in its priority ordering. Sinceapplication/javascript
has the highest priority only that gets rendered and doesn't end up finding the HTML tags it's meant to render into.The only approach I could possible see how to make it work is to dynamically create the HTML DOM nodes that the JS renders into. So this PR adds a bit of JS code to each plot that includes
application/javascript
, which looks for an existing DOM element with the matchingelement_id
and if that doesn't exist it dynamically creates it on its own parent node. This means that in the notebook nothing changes but in a static HTML export the DOM nodes are appropriately created.I have to say I really dislike this approach but it is absolutely the only way I see to make this possible, since I can only publish a single MIME type in the nbconvert output and
application/javascript
will always take precedence.