Skip to content

Implement lazy autoscaling in mplot3d. #18127

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 25, 2020
Merged

Implement lazy autoscaling in mplot3d. #18127

merged 3 commits into from
Dec 25, 2020

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Jul 30, 2020 •
edited
Loading

PR Summary

Since Axes3D derives from 2D Axes, this lazy autoscale was partially implemented. This might cause extraneous re-scaling since the 3D case did not always turn it off. It might also cause re-scaling to be missed since 2D Axes does not check z.

Fixes #18112.

This is a regression in 3.2.0 when lazy autoscaling was put in, but it's half a new feature, so I'm not sure if it belongs in 3.3.1. I should probably figure out how to update a test for this, and write an API change to match the 2D case.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • [n/a] New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • [n/a] Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

NilaiVemula
NilaiVemula reviewed Jul 30, 2020
View reviewed changes
Copy link

@NilaiVemula NilaiVemula left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes seem reasonable. There might be a better way to write this, though. I am not sure exactly how we can re-write this, but we should avoid repetitive code. Most of the new functionality is the same thing, just written three times for each axis.

@QuLogic QuLogic force-pushed the mplot3d-autoscale branch from 25986cd to 8e2c25b Compare July 31, 2020 02:48
@QuLogic
Copy link
Member Author

QuLogic commented Jul 31, 2020

Note, doc failures are due to #18133.

@QuLogic QuLogic force-pushed the mplot3d-autoscale branch from 8e2c25b to 7952e89 Compare August 29, 2020 05:45
@QuLogic QuLogic added this to the v3.4.0 milestone Aug 29, 2020
@QuLogic
Implement lazy autoscaling in mplot3d.
cfb046e 
Since `Axes3D` derives from 2D `Axes`, this lazy autoscale was partially
implemented. This might cause extraneous re-scaling since the 3D case
did not always turn it off. It might also cause re-scaling to be missed
since 2D `Axes` does not check z.

Fixes matplotlib#18112.
@QuLogic
TST: Parametrize 2D test_unautoscale also.
cd843e0
@QuLogic
DOC: Add an API note lazy autoscaling for Axes3D.
14d391e
efiring
efiring approved these changes Dec 25, 2020
View reviewed changes
Copy link
Member

@efiring efiring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
I checked that the test code from #18112 works interactively.

@efiring efiring merged commit 86f2d7b into matplotlib:master Dec 25, 2020
@QuLogic QuLogic deleted the mplot3d-autoscale branch December 25, 2020 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NilaiVemula NilaiVemula NilaiVemula left review comments

@efiring efiring efiring approved these changes

@tacaswell tacaswell tacaswell approved these changes

Assignees
No one assigned
Labels
topic: mplot3d
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.

set_{x,y,z}bound 3d limits are not persistent upon interactive rotation
4 participants
@QuLogic @efiring @tacaswell @NilaiVemula