-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Improved documentation for quiver #28863
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
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.
Please also fix the flake8 issues.
@Kaustbh you've requested a review, but have not reacted to my previous comments. They still stand. |
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.
Not your fault, but I believe the scale
and scale_units
behavior and documentation is far from clear: How do they interact?
- The behavior for both given is clear to me, just to be explained nicely.
- If both are absent, we do the autoscaling.
But what happens when either one is not given.
If we want to touch the scale
and scale_units
docs, we need a better understanding. This would require digging into the code and trying out the different cases. I don't want to urge you into this. Fiddling this out would be very welcome, but I also understand if you just wanted to do a small doc PR and not start to reverse engineer our code.
lib/matplotlib/quiver.py
Outdated
Number of data units per represented by one unit of arrow length on the plot. | ||
For example, if the data represents velocity in meters per second (m/s), the | ||
scale parameter determines how many meters per second correspond to one unit of | ||
arrow length relative to the width of the plot. | ||
Smaller scale parameter makes the arrow longer. Default is *None*. |
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 not an expert on the quiver logic. What I can say though from a quick look at the code is that
-
If
scale
is not not given, some autoscaling happens, so that we have "reasonable" arrow sizes. This should be mentioned. - I haven't determined exactly under which conditions (i.e.scale_units
settings). -
I also find the "units" framing and the physics example rather distracting. I think the relevant terms to connect here are "data values" U, V, (or "data length" if you will as sqrt(U2 +V2)), "scale_units" and scale. Basically this is done in the
scale_units
docs:
e.g. scale_units is 'inches', scale is 2.0, and
(u, v) = (1, 0)
, then the vector will be 0.5 inches long.
could have more context, but basically, it feels that discussing the scale in the context of scale_units makes more sense.
lib/matplotlib/quiver.py
Outdated
If the *scale* kwarg is *None*, the arrow length unit is automatically chosen based | ||
on scale_units. Default is *None*. |
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.
This is a topic for the scale
docs above.
matplotlib/lib/matplotlib/quiver.py Line 609 in 54d718e
The logic for scale and scale_units is implemented here, I will try to look into the code and fix the documentation.
|
Hi, there are different formuals for calculating the scaling factor of arrow length based on the |
You shouldn't necessarily document the formulas (as equation), but describe the logic; and for the logic, yes we need all cases - though you may group them together as e.g. "width" / "height" are analogous. |
is it okay? |
It would be simpler to review to put this into a commit, then I could annotate / suggest on individual parts.
It's not immedately obvious from the code, but I looks like this is effectively the same as "width". Or am I missing something?
The vector depends on U, V data values which are not specified here. Does it help to specify the data length "For example, if scale_units is 'width' or 'height' and scale is 1, a data length More generally, is this formula a good representation: In the above example we would get
I suggest not to go into the details of the algorithm. The description is not accurate enough, e.g. One important aspect to mention is that setting |
Hi, is the length we are getting from
for the above code, I am getting the length as [37.71236166], and the plot I am getting is this, the arrow in the plot has coordinates (0,0) -> (2,4.5), so how the length of arrow which we got is equal to the arrow length from plot? |
I believe this is in arrow-width units, i.e. the arrow is 37.7 times longer than wide, but t.b.c. i.e. try modifying |
Hi after reading the code and trying the different inputs I have made a commit, in this I have added a generalized formula for the different units, I also got that when scale_units is none it behaves like 'width'. and for the case of 'inches' in the given example, I do not understand how the 0.5 inches are shown on the plot. I'm sorry if there is any mistake in the Please tell me if it is fine or need more changes. |
lib/matplotlib/quiver.py
Outdated
scale_units : {'width', 'height', 'dots', 'inches', 'x', 'y', 'xy'}, optional | ||
If scale_units is None, the length of the arrows is effectively same as when it | ||
is set to 'width', meaning the arrow length will be relative to the width of | ||
the axes. |
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.
From a user perspective, this is effectively
scale_units : {'width', 'height', 'dots', 'inches', 'x', 'y', 'xy'}, optional | |
If scale_units is None, the length of the arrows is effectivel 8000 y same as when it | |
is set to 'width', meaning the arrow length will be relative to the width of | |
the axes. | |
scale_units : {'width', 'height', 'dots', 'inches', 'x', 'y', 'xy'}, default: 'width' |
(even though it's not obvious from the implementation, which could be a follow-up cleanup action)
lib/matplotlib/quiver.py
Outdated
- 'width' or 'height': The arrow length is scaled relative to the width or height | ||
of the axes. In this case, the arrow length will be relative to the width or | ||
height of the axes. |
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.
- 'width' or 'height': The arrow length is scaled relative to the width or height | |
of the axes. In this case, the arrow length will be relative to the width or | |
height of the axes. | |
- 'width' or 'height': The arrow length is scaled relative to the width or height | |
of the Axes. |
Both sentences say the same thing. Note that we capitalize "Axes" when refering to the logical construct and not just a plural of axis.
lib/matplotlib/quiver.py
Outdated
"For example, if *scale_units* is 'width' or 'height' and *scale* is 1.0, will | ||
result in an arrow length of width/height of the axes." |
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.
"For example, if *scale_units* is 'width' or 'height' and *scale* is 1.0, will | |
result in an arrow length of width/height of the axes." | |
"For example, ``scale_units='width', scale=1.0``, will result in an arrow length | |
of width of the Axes." |
It's sufficient to choose one of width/height for an example. Also, I think stating all relevant parameters
as one code expression is slightly more readable.
is it correct for the case of
|
I'm unclear what "it" is. Anyway, #28863 (comment) cover 'x' and 'y' cases as well. |
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.
Minor formatting improvements. It's ready to go after addressing those.
lib/matplotlib/quiver.py
Outdated
units. To plot vectors in the x-y plane, with u and v having | ||
the same units as x and y, use | ||
``angles='xy', scale_units='xy', scale=1``. | ||
Note: setting scale_units without setting scale does not have any effect because |
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.
Note: setting scale_units without setting scale does not have any effect because | |
Note: Setting *scale_units* without setting scale does not have any effect because |
Hi, are there any changes to be made? |
Yes, but they were due to currently brittle handling of missing code references. I've made the relevant changes, so that you don't have to bother with this unrelated problem. |
PR summary
Issue #28209. This pull request improves the documentation for several key parameters in the
quiver
function of Matplotlib. The goal is to make it easier for users to understand how these parameters work and how they affect the behavior of quiver plots.