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

Matplotlib aspect fixes #1209

Merged
merged 5 commits into from Mar 16, 2017

Conversation

Projects
None yet
3 participants
@philippjfr
Member

philippjfr commented Mar 15, 2017

Fixes #199, #996 and #1144. Plots in Layouts are no longer forced to square, semilog/loglog plots correctly apply aspects, and axis ranges are set after ticks.

@philippjfr philippjfr added the mpl label Mar 15, 2017

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 15, 2017

Good to see three issues sorted with so little code! I guess the test data will now need to be updated...

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 15, 2017

Maybe, don't know yet, made a typo.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 16, 2017

Continuous Coordinates

Before:

image

Now:

image

Elements

Before:

image

Now:

image

Sampling Data

Before:

image

Now:

image

I also still need to make sure that polar plots are forced to be square.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 16, 2017

I think the original reason we tried to force everything to be square is that it made layouts easier.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 16, 2017

I think the original reason we tried to force everything to be square is that it made layouts easier.

Absolutely and without the layout fix that I added these examples would have really odd spacing I'm sure.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 16, 2017

Ready to merge whenever, rebuilt the test data and PR build is passing.

@jbednar

This comment has been minimized.

Member

jbednar commented Mar 16, 2017

Excellent; thanks so much for fixing this previously very surprising behavior. That said, even though these are surely reasonable to consider as bug fixes, they will affect the look and feel of existing notebooks, so they should be mentioned under compatibility changes. Basically, if someone was relying on Layout to make things square, they will now have to force them to be square, and it will be helpful to mention how to do that in the release notes.

@philippjfr philippjfr added this to the v1.7.0 milestone Mar 16, 2017

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 16, 2017

Sorry if I missed it but that is a good point - is there any easy way to restore the old behavior if necessary?

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 16, 2017

Not really no, except setting hv.plotting.mpl.RasterPlot = 'square', which will also affect plots not in Layouts. You can always explicitly set %%opts Image [aspect='square'] though.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 16, 2017

Ok, then as Jim said it will need to be mentioned in the release notes.

The pr build is passing, is it ready to merge?

@philippjfr

This comment has been minimized.

Member

philippjfr commented Mar 16, 2017

Yes, ready.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Mar 16, 2017

Great. Merging.

@jlstevens jlstevens merged commit e8fa25c into master Mar 16, 2017

2 of 4 checks passed

continuous-integration/travis-ci/push The Travis CI build failed
Details
s3-reference-data-cache Test data not cached, see details to rebuild.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 78.426%
Details

@philippjfr philippjfr deleted the mpl_aspect_fixes branch Apr 11, 2017

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