Move label hiding rectilinear-only check into _label_outer_{x,y}axis. #21317
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
anntzer
commented
Oct 8, 2021
| @@ -396,10 +396,15 @@ def test_remove_shared_polar(fig_ref, fig_test): | |||
|
|
|||
| def test_shared_polar_keeps_ticklabels(): | |||
| fig, axs = plt.subplots( | |||
| 2, 2, subplot_kw=dict(projection="polar"), sharex=True, sharey=True) | |||
| 2, 2, subplot_kw={"projection": "polar"}, sharex=True, sharey=True) | |||
There was a problem hiding this comment.
this is just to make things fit in 79 characters in the equivalent expression below.
|
I'm pro making the change in #19994, since that's a function that sets up axes, but I'm against making the change here:
|
|
I agree with your points. The original motivation was to prepare work on #18305. However, I changed this PR on two points to address your comments:
|
This allows having it both in subplots() (which had that check before) and in subplot_mosaic (which didn't, see test). (All Axes returned by subplots() have the same projection, so `all(ax.name == "rectilinear")` can be replaced by independently checking each Axes.) Also improve the rectilinearity check by testing the shape of the axes patch instead (so that e.g. skewT also participates in outer-only-labels), and add an escape hatch so that Axes.label_outer always does "what it says on, the tin".
|
This reverts a bit of #19994, in that it's disabled only for the relevant Axes and not all. Since that's not released, should we trying to put this (or something similar) in 3.5? |
|
I'm not sure how many people this actually affects, but yes, this prevents a regression on skewT-type shared axes in 3.5, so if you can still squeeze this in, that would be a good idea. |
QuLogic
approved these changes
Nov 2, 2021
meeseeksmachine
pushed a commit
to meeseeksmachine/matplotlib
that referenced
this pull request
Nov 11, 2021
…k into _label_outer_{x,y}axis.
tacaswell
added a commit
that referenced
this pull request
Nov 12, 2021
…317-on-v3.5.x Backport PR #21317 on branch v3.5.x (Move label hiding rectilinear-only check into _label_outer_{x,y}axis.)
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.
This allows having it both in subplots() (which had that check before)
and in subplot_mosaic (which didn't, see test). (All Axes returned by
subplots() have the same projection, so
all(ax.name == "rectilinear")can be replaced by independently checking each Axes.)
xref #19994, again this may mess up some third-party axes (#19994 (comment)) but the tradeoff seemed OK back then. (We can always consider adding a "does this want to participate in label_outer" flag if there's demand for it, but let's not overengineer things for now.)
Edit, following review: Also improve the rectilinearity check by testing the shape of the axes
patch instead (so that e.g. skewT also participates in
outer-only-labels), and add an escape hatch so that Axes.label_outer
always does "what it says on, the tin".
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).