-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
quiver angles parameter docstring nitpick #28840
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
Conversation
In the docstring, the sentence read "Use this ... if NOT", which is not good sentence structure. If you are going to negate a "Use this ...", the sentence should read "Do not use this if ...". However, sentences in documentation should avoid negatives because they require mental effort to logically negate. If negatives are being used, the sentence should start with it, like "DO NOT". I fixed the error in this pull documentation update so that it avoids the problematic sentence structure that I mentioned earlier. I made both cases of the docs much more explicit.
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.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
There was a proble 8000 m hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrapped comment
@@ -88,13 +88,14 @@ | |||
Method for determining the angle of the arrows. | |||
|
|||
- 'uv': Arrow direction in screen coordinates. Use this if the arrows | |||
symbolize a quantity that is not based on *X*, *Y* data coordinates. | |||
symbolize normalized x-y components. |
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 think this is clear. First, what does "normalized" mean here? It's a term with multiple connotations. Second, mentioning "x-y" is a red herring, because it's close to X and Y, which we specifically don't want to associate here.
If don't find the original wording too bad. At it's core there is a negation: "all data that are not related to the data coordinates". You could try to work around the "not" by
that is unrelated to X, Y data coordinates.
but 🤷
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 of my engineering background, I thought "normalized" was a standard word. My bad. Maybe something like unitary?
The descriptions have been updated as part of a larger overhaul of the quiver docs in #28863. Please check whether the new wording is sufficient or whether there is still need for improvement: https://matplotlib.org/devdocs/api/_as_gen/matplotlib.axes.Axes.quiver.html |
PR summary
In the docstring, the sentence read "Use this ... if NOT", which is not good sentence structure. If you are going to negate a
"Use this ...", the sentence should read "Do not use this if ...".
Furthermore, sentences in documentation should avoid negatives because they require mental effort to logically negate. If negatives are being used, the sentence should start with it, like "DO NOT". The one exception to this rule, is if something is commonly used one way, then "Do not" makes sense to warn the user against auto-pilot.
I fixed the error in this pull documentation update so that it avoids the problematic sentence structure that I mentioned earlier. I made both cases of the docs much more explicit.
PR checklist