Added better support for computing bokeh plot size #1140
Conversation
|
Ready for review and merge. |
holoviews/plotting/bokeh/renderer.py
Outdated
| width, height = plot.plot_width, plot.plot_height | ||
| elif isinstance(plot, (Plot, Div, DataTable)): | ||
| width, height = plot.width, plot.height | ||
| if width is None: width = 0 |
There was a problem hiding this comment.
Isn't this an error condition? What are you going to do with a plot that is either zero width, zero height or both?
There was a problem hiding this comment.
Certain models do not declare a width/height and rather than guess at it I'd take the hit and just say it's zero. The only thing that I've found that's actually affected by this is the Div model used to add titles to grid plots.
There was a problem hiding this comment.
Perhaps I'll just add an explicit branch for the Div model, and make a comment about this.
There was a problem hiding this comment.
Perhaps I'll just add an explicit branch for the Div model, and make a comment about this.
Sounds like a good idea.
|
I made one comment (although I can't say I follow all the detailed size computation logic!) but generally it looks good. The main thing I checked was that Happy to merge once you address my comment. |
philippjfr
force-pushed
the
bokeh_renderer_get_size
branch
from
9eb240d
to
b594244
Compare
|
@jlstevens Ready to merge. |
|
Looks good. Merging. |
The current
BokehRenderer.get_sizemethod only supports very simple plots that are not part of a layout grid, or contain toolbars and tables. This PR computes the size of a bokeh plot more accurately for all these cases, which can be very useful when wanting to embed the plot in HTML and know the size ahead of time.