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

Compatibility with bokeh layout refactor #3450

Merged
merged 32 commits into from
Mar 23, 2019
Merged

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Feb 1, 2019

This PR achieves compatibility with the layout refactor which was merged into bokeh dev and will land in bokeh 1.1.0. It will give us control over the actual plot frame dimensions (excluding the axes, colorbars, legends) and provide control over the plot aspect.

  • Adds support for aspect and data_aspect in bokeh (Separating options to set figure vs. data aspect ratio #335)
  • Fixes support for sizing modes in bokeh
  • Adds lower level control over reactive layouts using min/max_width/height and width/height_policy options
  • Ensure data aspect is maintained when updating plot
  • Ensure datetime and categorical axes warn if data_aspect set
  • Test on bokeh server
  • Check no events bounce on datashade
  • Update documentation
  • Update tests
  • Add tests

Fixes #3026
Fixes #2992

@philippjfr
Copy link
Member Author

philippjfr commented Feb 14, 2019

The main thing to settle here is how we should handle the switch from widths/heights which include axes/colorbars/legends to frame widths which apply only to the plot itself.
The original behavior makes it very easy to get multiple plots to align nicely so it still has uses and we don't want to mess with everyones existing layouts without at least some warning so I think there are two options I can think of:

  1. Add an option to switch between frame size and overall size (and for 2.0 we could consider switching the default to the frame width/height)
  2. Add separate frame_width/frame_height options which override the standard width/height

Edit: Consensus here was to use option 2 and add separate frame_ options which take precedence if set.

@philippjfr
Copy link
Member Author

philippjfr commented Feb 23, 2019

I'm going to make an attempt at explaining the width, height, aspect and data_aspect behavior and explain how they interact with the plot dimensions and ranges particularly as it relates to responsive behavior and updating plots.

width/height/frame_height/frame_width

width/height: Specifies the dimensions of the plot as a whole including legends/axes/colorbars/titles

frame_width/frame_height: Specifies the dimensions of just the plot frame

These options take absolute precedence over everything, if a fixed height or width is specified by a user no other option including aspect, data_aspect or responsive will have an effect (and will warn). A frame_width/frame_height will take precedence over a regular width/height.

Aspect

The aspect specifies the ratio between the width and height dimensions of the plot. If specified this options takes absolute precedence over the dimensions of the plot but has no effect on the plot ranges. The only exception is aspect='equal' which is equivalent to data_aspect=1. Aspect will only take effect if either or both width and height are undefined.

Data aspect

The data_aspect specifies the scaling between the x- and y-axis ranges. If specified this option will scale both the plot ranges and dimensions unless an aspect value overrides the plot dimensions.

Responsive

In responsive mode the plot size rescales to the size of its container depending on the other options that were specified. responsive maps to bokeh's sizing mode as follows:

  • stretch_both: Stretches plot to maximum available width and height if no width, height or aspect are provided.
  • stretch_width: Stretches plot to maximum available width if no width and no aspect are provided
  • stretch_height: Stretches plot to maximum available height if no height and no aspect are provided
  • scale_both: Scales both width and height of plot while maximizing the width of the plot, if aspect is defined but no fixed width/height are.
  • scale_width: Scales both width and height of plot while maximizing the width of the plot, if aspect is defined but no fixed width/height are and if responsive='width' (rarely used).
  • scale_height: Scales both width and height of plot while maximizing the height of the plot, if aspect is defined but no fixed width/height are and if responsive='width' (rarely used).

There is one major caveat when combining a data aspect and responsive mode, in this case we cannot fully guarantee that a specified data aspect is maintained. This is because in responsive mode we don't have control over the plot frame dimensions just the overall plot aspect. We may be able to alleviate this by writing some JS code that maintains equal aspect in the plot ranges or by dynamically syncing the plot frame extents.

Updating Plots

The other major issue here is maintaining a fixed aspect while a plot is updating. We do not want a plot resizing when you update it (in fact I could not even get that working), so the only way to achieve a fixed data_aspect is adjusting the ranges when new data comes in that has a different aspect. This works well but suffers from the same issue described above when responsive is enabled.

@philippjfr philippjfr force-pushed the bokeh_layout_refactor branch 4 times, most recently from 72a9e68 to b8db63b Compare March 1, 2019 02:26
@philippjfr philippjfr force-pushed the bokeh_layout_refactor branch 2 times, most recently from aac3a41 to 911ed98 Compare March 11, 2019 02:01
@jbednar
Copy link
Member

jbednar commented Mar 15, 2019

I know we discussed all of the above comments at some point, but my brain starts to hurt when trying to really think deeply about them now. That said, as far as I can see that all sounds like what we had discussed and seems like the best approach in each case.

@jlstevens
Copy link
Contributor

I agree with Jim that this sounds just like we discussed. From the description, it sounds like everything behaves the way I would hope, except maybe for the caveat about data_aspect.

@ea42gh
Copy link
Contributor

ea42gh commented Mar 15, 2019

Feature request: plot_size option for GridSpace should support tuples

@jlstevens
Copy link
Contributor

@philippjfr Looks great! Only had one minor request (docstring improvement) and other than that, I just want a way to remember to remove -c bokeh/label/dev once the new bokeh version is released.

Happy to merge once the small change is pushed (no need to wait on the tests this time).

@jlstevens
Copy link
Contributor

Great thanks! Merging.

@jlstevens jlstevens merged commit b1c28f1 into master Mar 23, 2019
@philippjfr philippjfr deleted the bokeh_layout_refactor branch April 29, 2019 11:24
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show legend in gridmatrix GridSpace shared_xaxis/shared_yaxis with non-default height/width
4 participants