Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Update and factor out Axis.get_tick_positions.#13335
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
lib/matplotlib/axis.py Outdated
| the first major and the first minor ticks, and return | ||
| - 1 if only tick1line and label1 are visible; | ||
| - 2 if only tick2line and label2 are visible; |
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.
comment here why these are left ambiguous....
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.
what do you mean?
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.
Why they don’t return strings.
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.
Because they become "top"/"bottom" for the x-axis but "left"/"right" for the y-axis? For an abstract Axis, 1 and 2are the names of the ticks and labels, they don't have string names yet.
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 understand, but it took me a bit, and it will be even more mysterious for someone who doesn't have the diff to look at. Just adding a one-line comment here would make it clearer...
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 it would be useful to clarify that 1 means "top" or "bottom" and 2 means "left" or "right" in the docstring here.
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.
done
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.
👍 on code, docstring could be clarified though
lib/matplotlib/axis.py Outdated
| the first major and the first minor ticks, and return | ||
| - 1 if only tick1line and label1 are visible; | ||
| - 2 if only tick2line and label2 are visible; |
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 it would be useful to clarify that 1 means "top" or "bottom" and 2 means "left" or "right" in the docstring here.
YAxis.get_ticks_position had not been updated after the deprecation oflabel1On. Fix that by making it share its implementation withXAxis.get_ticks_position.
YAxis.get_ticks_position had not been updated after the deprecation of
label1On (#10088). Fix that by making it share its implementation with
XAxis.get_ticks_position.
Release critical as YAxis.get_ticks_position otherwise emits a DeprecationWarning.
PR Summary
PR Checklist