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

Fixed handling of shared_axes #1187

Merged
merged 2 commits into from Mar 10, 2017

Conversation

Projects
None yet
2 participants
@philippjfr
Member

philippjfr commented Mar 10, 2017

The bokeh backend supports a shared axes option which allows axes with the same axis labels to be linked. The current approach taken here is not quite correct because the plot doesn't remember that an axis is shared so it will simply override extents set by a different plot. Secondly setting x/yaxis='bare' will currently disable shared_axes because it removes the axis label.

While this behavior is a lot more consistent, we might want to consider disabling it by default in some cases (e.g. on a simply Layout) because it can interfere with axiswise normalization.

plot_ranges['y_range'] = plot.y_range
if plot.yaxis[0].axis_label == xlabel:
plot_ranges['x_range'] = plot.y_range
return plot_ranges

This comment has been minimized.

@jlstevens

jlstevens Mar 10, 2017

Member

I'm a little confused here...there are only two keys 'x_range' and 'y_range' so if you iterate over a bunch of plots can't you end up clobbering these keys with different values? Won't you end up with whatever was found at the end of the loop?

This comment has been minimized.

@philippjfr

philippjfr Mar 10, 2017

Member

That is correct, I could have two separate loops for the x- and y-axis and break when a matching axis is found. However in practice if shared_axes are enabled all the plots will share the same axes anyway so it doesn't much matter whether I end up using the axis from the first or last plot, because they are all the same anyway.

This comment has been minimized.

@jlstevens

jlstevens Mar 10, 2017

Member

Ok, good that you've thought about this. It is also good to have this pulled out as a method.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 10, 2017

Looks good and thanks for adding the unit tests.

Merging.

@jlstevens jlstevens merged commit 51d4b09 into master Mar 10, 2017

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 increased (+0.02%) to 78.455%
Details
s3-reference-data-cache Test data is cached.
Details

@philippjfr philippjfr deleted the shared_axes branch Apr 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment