-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Fix get_tick_params #27408
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix get_tick_params #27408
Conversation
a89696f to
4a313a6
Compare
| Width of gridlines in points. | ||
| grid_linestyle : str | ||
| Any valid `.Line2D` line style spec. | ||
| gridOn : bool |
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'm hesitant to further expand the use of camelcase variables. But I don't have an overview of its usage and whether grid_on would be a viable alternative. (No code editor access right now).
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.
grid_on would be available as an alternative. gridOn is only used internally in 4 files, no big deal to rename.
As an alternative one could translate gridOn to grid_on (which wouldn't, however, prevent anyone to use the then undocumented keyword in set_tick_params etc., as it's the case now).
If we don't want to have gridOn I'd opt for renaming it to grid_on. This wouldn't be an API change as gridOn is not documented.
| @@ -0,0 +1,8 @@ | |||
| Classes derived from Axis must implement get_tick_params | |||
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.
Wouldn't it be sufficient to implement the translation, either as a dict or as _translate_tick_params()?
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.
Sure, but _translate_tick_params is 18 LOC (not counting continuation lines) whereas get_tick_params is just 2.
Another thing is that set/get_tick_params currently doesn't work correctly for Axes3D (I'm going to open an issue about it) and I assume that it's easier fix with dedicated setters/getters (didn't look into it closely 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.
On second thought, I think you're right and it's less convoluted to make the translation dicts axis-specific in the first place rather than modify a generic dict. I'll change it later this week, changing this PR to draft in the meantime.
- show values for x axis too - fix nonfunctional test - move axis tests from test_axes.py to test_axis.py - make note in get_tick_params docstring more precise - add GridOn to set_tick_params as it's always returned by get_tick_params, so it would be illogical if you couldn't set the paramter in the setter
4a313a6 to
538f2eb
Compare
|
This re-write of the Note in the get_tick_params documentation was caused by this example: import matplotlib.pyplot as plt
import matplotlib as mpl
with mpl.rc_context({"ytick.labelcolor": "blue"}):
fig, ax = plt.subplots()
for label in ax.get_xticklabels():
label.set_fontweight("bold")
plt.xticks(ha="left")
fig.text(0.2, 0.7, "\n".join(["___x___"] + [f"{k}: {v}" for k, v in ax.xaxis.get_tick_params().items()]), va="top")
fig.text(0.6, 0.7, "\n".join(["___y___"] + [f"{k}: {v}" for k, v in ax.yaxis.get_tick_params().items()]), va="top")
plt.show()This shows that
|
| .. note:: | ||
| This method only returns the values of the parameters *bottom*, *top*, | ||
| *labelbottom*, *labeltop* or *left*, *right*, *labelleft*, *labelright*, | ||
| respectively, and *girdOn* as well as all additional parameters that were |
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.
| respectively, and *girdOn* as well as all additional parameters that were | |
| respectively, and *gridOn* as well as all additional parameters that were |
|
I believe this has been superseeded by #29249. |

PR summary
Fix get_tick_params
get_tick_paramsdocstring more preciseGridOntoset_tick_paramsas it's always returned byget_tick_params, so it would be illogical if you couldn't set the paramter in the setterCloses #27416.
PR checklist