Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Fix get_tick_params#27408
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
| @@ -3394,6 +3394,8 @@ def tick_params(self, axis='both', **kwargs): | |||
| 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 whethergrid_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 translategridOn togrid_on (which wouldn't, however, prevent anyone to use the then undocumented keyword inset_tick_params etc., as it's the case now).
If we don't want to havegridOn I'd opt for renaming it togrid_on. This wouldn't be an API change asgridOn 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) whereasget_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
This re-write of the Note in theget_tick_params documentation was caused by this example: importmatplotlib.pyplotaspltimportmatplotlibasmplwithmpl.rc_context({"ytick.labelcolor":"blue"}):fig,ax=plt.subplots()forlabelinax.get_xticklabels():label.set_fontweight("bold")plt.xticks(ha="left")fig.text(0.2,0.7,"\n".join(["___x___"]+ [f"{k}:{v}"fork,vinax.xaxis.get_tick_params().items()]),va="top")fig.text(0.6,0.7,"\n".join(["___y___"]+ [f"{k}:{v}"fork,vinax.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*aswellasalladditionalparametersthatwere | |
| respectively,and*gridOn*aswellasalladditionalparametersthatwere |
I believe this has been superseeded by#29249. |
Uh oh!
There was an error while loading.Please reload this page.
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