Factor out common limits handling for x/y/z axes. #21442
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
f840d22 to
e463b79
Compare
timhoffm
reviewed
Dec 4, 2021
jklymak
reviewed
Feb 4, 2022
| *xmax* respectively; *emit* and *auto* are used as is. | ||
| """ | ||
| v0name, v1name = names # The value names. | ||
| name, = [name for name, axis in self.axes._get_axis_map().items() |
There was a problem hiding this comment.
This seems a really obscure way to get the name to me. Is there some reason an axis can't know its "name" when it is created?
There was a problem hiding this comment.
Axises know their "name", but that's e.g. "r"/"theta" for polar axes whereas here we really want "x"/"y" even for polar. Perhaps Axises should have multiple name attributes for the various use cases, but that would be a bit hairy too...
jklymak
reviewed
Feb 4, 2022
|
Rebased. |
jklymak
approved these changes
Feb 4, 2022
timhoffm
reviewed
Feb 4, 2022
timhoffm
reviewed
Feb 4, 2022
112c294 to
56cf49d
Compare
timhoffm
reviewed
Feb 5, 2022
6 tasks
timhoffm
approved these changes
Feb 6, 2022
lib/matplotlib/axis.py
Outdated
| # rather than generically using set_view_interval, so that shared | ||
| # axes get updated as well. | ||
| raise NotImplementedError('Derived must override') | ||
| # docstring inherited |
There was a problem hiding this comment.
You can't have that comment and a docstring. :)
The change in set_top_view is necessary because super().set_xlim no
longer goes to 2D semantics (it's on the Axis.set_view_limits that one
would need to do a supercall), but in any case the old behavior was
actually more buggy because it would e.g. confuse data-limits sharex and
2D projection sharex; e.g.
`subplots(1, 2, sharex=True, subplot_kw={"projection": "3d"})`
was previously wrong.
As a side effect, also fixes `invert_yaxis`/`invert_zaxis` on 3d axises
(because they just go to the base `Axis.set_inverted`).
|
Doc failures are real. |
The "canonical" property names for Axes3D were switched from xlim3d/ylim3d to xlim/ylim so that Axes3D can directly inherit the setters from 2D Axes.
|
Test failure should be fixed now; foc failures seem related to the new scipy 1.8 release? |
|
This broke docs (see failed build in #22431): |
6 tasks
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The change in set_top_view is necessary because super().set_xlim no
longer goes to 2D semantics (it's on the Axis.set_view_limits that one
would need to do a supercall), but in any case the old behavior was
actually more buggy because it would e.g. confuse data-limits sharex and
2D projection sharex; e.g.
subplots(1, 2, sharex=True, subplot_kw={"projection": "3d"})was previously wrong.
As a side effect, also fixes
invert_yaxis/invert_zaxison 3d axises (#21369)(because they just go to the base
Axis.set_inverted). Redoing the inheritancehierarchy (#21424) would probably(?) still be nice, at least from a semantic PoV.
old bad 3d sharex:
PR Summary
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).