Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
AxisArtist to use standard tick directions and have an option for tick orientation#24553
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
base:main
Are you sure you want to change the base?
Conversation
i.e. defaulting to "out". Also support "inout".Would perpendicular work better for "out"? see e.g.demo_curvelinear_grid.
| else "parallel") | ||
| return tick_orientation | ||
| def set_tick_out(self, b): |
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.
Maybe we should deprecate these now? (set/get_tick_out)
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.
At what version do we want it to be deprecated?
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.
I'd say 3.7.
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.
I think this could fit in our normal warn for one release, remove the next pattern, but would defer to you if you wanted to put the new version in for a cycle before we started (or pushed the removal date out further).
| # If ticklabels and axislabel are on differenct side, we only | ||
| # conside the padding for the ticks only. | ||
| ticksizes = [] |
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.
Can you add a test that uses this path? (And ideally "all" the newly introduced options.)
Edit: one way to at least test theout/in/inout combinations would be to regenerate some of the test images below.
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.
Yes, I will do.
| @@ -21,6 +21,9 @@ def test_subplot(): | |||
| # remove when image is regenerated. | |||
| @image_comparison(['curvelinear3.png'], style='default', tol=5) | |||
| def test_curvelinear3(): | |||
| # Remove this lines when this test image is regenerated. | |||
| plt.rcParams.update({"xtick.direction": "in", "ytick.direction": "in"}) | |||
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.
If I get it correct, this changes the current behavior? Not sure if we should introduce some way to make that transition smoothly, but at least a change note would be in place, I guess.
(Edit: this comment holds for the whole change, not just this line.)
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.
When we went to v2 we changed ticks from in to out, however because AxisArtist did not follow the rcparams they did not flip when everything else did.
I do not think there is a way to soften this change short of adding another rcparam.
I think we should go forward with this, but make sure there is a very clear API change note.
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.
I have added a deprecation warning at the level of Tick creation and relevant tests. On the other hand, users won't see the warning unless they manually create a Tick instance. I briefly thought about adding atickdir keyword argument for Axes initialization, but not very inclined for now. I agree with@tacaswell that it is better to go with it.
| @@ -62,7 +62,7 @@ | |||
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.
The line above (which I cannot mark...) should be updated as well?
Currently:
ax.axis["bottom"].major_ticks.set_tick_out(True)
PR Summary
This is a rebased version of PR#19102 that addresses issue#19101. It also add an option to change the orientation of ticks. The ticks in AxisArtist were drawn along the gridlines and may not be perpendicular to spines. This PR introduce an option of
tick_orientationthat controls this behavior so that it can be normal to spines (see the discussion in#19102).tick.orientationcan be one of "parallel", "normal" and "auto". Defaut is "auto".PR Checklist
Documentation and Tests
pytestpasses)Release Notes
.. versionadded::directive in the docstring and documented indoc/users/next_whats_new/.. versionchanged::directive in the docstring and documented indoc/api/next_api_changes/next_whats_new/README.rstornext_api_changes/README.rst