Skip to content

Implement lazy autoscaling in mplot3d.#18127

Merged
efiring merged 3 commits intomatplotlib:masterfrom
QuLogic:mplot3d-autoscale
Dec 25, 2020
Merged

Implement lazy autoscaling in mplot3d.#18127
efiring merged 3 commits intomatplotlib:masterfrom
QuLogic:mplot3d-autoscale

Conversation

@QuLogic
Copy link
Copy Markdown
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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
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

@efiring efiring efiring approved these changes

@tacaswell tacaswell tacaswell approved these changes

+1 more reviewer

@NilaiVemula NilaiVemula NilaiVemula left review comments

Reviewers whose approvals may not affect merge requirements

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