-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Capitalize some docstrings. #13108
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
Capitalize some docstrings. #13108
Conversation
@@ -1356,8 +1356,7 @@ def rgb_to_hsv(arr): | |||
|
|||
def hsv_to_rgb(hsv): | |||
""" | |||
convert hsv values in a numpy array to rgb values | |||
all values assumed to be in range [0, 1] |
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 same point is documented two lines below
lib/matplotlib/textpath.py
Outdated
verts, codes = font.get_path() | ||
if currx != 0.0: | ||
cbook._warn_deprecated( | ||
"3.1", name="currx", obj_type="argument", | ||
alternative="Manually translate the path vertices.") |
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.
currx is not used in the codebase and that's really trivial for the caller to do it
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 really dislike slipping in deprecations in a commit that says it's about trivial changes to docstrings.
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.
Sorry about that, reverted.
lib/matplotlib/backend_tools.py
Outdated
@@ -853,7 +852,7 @@ def _cancel_action(self): | |||
return | |||
|
|||
def _press(self, event): | |||
"""the _press mouse button in zoom to rect mode callback""" | |||
"""Callback for mouse button presses in zoom to rect mode.""" |
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.
"""Callback for mouse button presses in zoom to rect mode.""" | |
"""Callback for mouse button presses in zoom-to-rect mode.""" |
I would use hyphens for better readability, but not a native speaker, so not sure that's correct.
(occurs multiple times also in docstrings 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.
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.
I don't see this?
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.
Hum, sorry, forgot to push. Done.
lib/matplotlib/textpath.py
Outdated
verts, codes = font.get_path() | ||
if currx != 0.0: | ||
cbook._warn_deprecated( | ||
"3.1", name="currx", obj_type="argument", | ||
alternative="Manually translate the path vertices.") |
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 really dislike slipping in deprecations in a commit that says it's about trivial changes to docstrings.
I agree with @QuLogic that we should separate documentation cleanup and functional changes. In particular, functional changes should be clear from the commit/PR message. It's not much work to do the deprecation in a separate PR. |
Apologies, reverted the deprecation. |
e9f6f22
to
5ad94a9
Compare
5ad94a9
to
ecefd71
Compare
Co-Authored-By: anntzer <anntzer.lee@gmail.com>
ecefd71
to
2585d9f
Compare
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.
Thanks @anntzer !
PR Summary
PR Checklist