Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Move label hiding rectilinear-only check into _label_outer_{x,y}axis.#21317
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| @@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 bysubplots() 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 axespatch instead (so that e.g. skewT also participates inouter-only-labels), and add an escape hatch so that Axes.label_outeralways 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. |
…k into _label_outer_{x,y}axis.…317-on-v3.5.xBackport PR#21317 on branch v3.5.x (Move label hiding rectilinear-only check into _label_outer_{x,y}axis.)
Uh oh!
There was an error while loading.Please reload this page.
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).