8000 Improved documentation for quiver by Kaustbh · Pull Request #28863 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 6 commits into from
Oct 26, 2024
Merged

Improved documentation for quiver #28863

merged 6 commits into from
Oct 26, 2024

Conversation

Kaustbh
Copy link
Contributor
@Kaustbh Kaustbh commented Sep 22, 2024

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.

Copy link
Member
@timhoffm timhoffm left a 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 Kaustbh requested a review from timhoffm September 27, 2024 18:42
@timhoffm
Copy link
Member

@Kaustbh you've requested a review, but have not reacted to my previous comments. They still stand.

@QuLogic QuLogic removed the request for review from timhoffm September 28, 2024 00:05
@Kaustbh Kaustbh requested a review from timhoffm September 28, 2024 06:03
Copy link
Member
@timhoffm timhoffm left a 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.

Comment on lines 125 to 129
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*.
Copy link
Member

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.

Comment on lines 136 to 137
If the *scale* kwarg is *None*, the arrow length unit is automatically chosen based
on scale_units. Default is *None*.
Copy link
Member

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.

@Kaustbh
Copy link
Contributor Author
Kaustbh commented Oct 2, 2024

def _make_verts(self, XY, U, V, angles):

The logic for scale and scale_units is implemented here, I will try to look into the code and fix the documentation.

@Kaustbh
Copy link
Contributor Author
Kaustbh commented Oct 5, 2024

Hi, there are different formuals for calculating the scaling factor of arrow length based on the scale_units, should I include all the formulas for different scale_units in documentation?

@timhoffm
Copy link
Member
timhoffm commented Oct 7, 2024

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.

@Kaustbh
Copy link
Contributor Author
Kaustbh commented Oct 10, 2024
scale : float, optional
    Scales the length of the arrow inversely.

    Number of data values 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.

    If *None*, a autoscaling algorithm is used, based on the average
    vector length and the number of vectors.
    The scaling factor is adjusted using the formula `*scale* = 1.8*amean*sn/self.span`, 
    where `amean` is the average arrow length, `sn` is the number of arrows, and `self.span`
    is the plots extent.
    The arrow length unit is given by the *scale_units* parameter.

scale_units : {'width', 'height', 'dots', 'inches', 'x', 'y', 'xy'}, optional
    If scale_units is None, the length of the arrows is determined directly by data values. 
    This means that the arrow length will correspond to the values in U and V without any 
    additional scaling based on figure dimensions or display units. 
    In this case, the arrow length is controlled purely by the scale parameter, and 
    no unit conversion is applied.

    - 'width' or 'height': The arrow length is scaled relative to the width or height
      of the axes. 
      For example, if *scale_units* is 'width' or 'height' and *scale* is 2.0, the
      vector will be half the width/height of the axes.
    
    - 'dots': The arrow length of the arrows is is measured in display dots (pixels).

    - 'inches': Arrow lengths are scaled based on the DPI (dots per inch) of the figure.
      This ensures that the arrows have a consistent physical size on the figure, in inches,
      regardless of data values or plot scaling.
      For example, if scale_units is 'inches', scale is 2.0, and `(u, v) = (1, 0)`, 
      then the vector will be 0.5 inches long.
    
    - 'x' or 'y': The arrow length is scaled relative to the x or y axis units.
      For example, if *scale_units* is 'x' and *scale* is 2.0, the vector will be 0.5
      x-axis units long.
    
    - 'xy': Arrow length is expressed in terms of the combined x and y data values.
      This is useful for creating vectors in the x-y plane where u and v have
      the same units as x and y. 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``.

is it okay?

@timhoffm
Copy link
Member

It would be simpler to review to put this into a commit, then I could annotate / suggest on individual parts.

If scale_units is None [...]

It's not immedately obvious from the code, but I looks like this is effectively the same as "width". Or am I missing something?

For example, if *scale_units* is 'width' or 'height' and *scale* is 2.0, the
vector will be half the width/height of the axes.

The vector depends on U, V data values which are not specified here. Does it help to specify the data length $l_{UV} = \sqrt{u^2 + v^2}$? I also suggest to use scale=1 if possible to simplify the calculation. Then you could say:

"For example, if scale_units is 'width' or 'height' and scale is 1, a data length $l_{UV}=0.5$ will result in an arrow length of width/height of the axes."

More generally, is this formula a good representation: $l_{arrow} = l_{UV} * \frac{\mathrm{scale\_unit}}{\mathrm{scale}}$?

In the above example we would get $l_{arrow} = 0.5 * \frac{\mathrm{Axes\ width}}{1} = 0.5 \mathrm{Axes\ width}$

The scaling factor is adjusted using the formula `*scale* = 1.8*amean*sn/self.span`, 
where `amean` is the average arrow length, `sn` is the number of arrows, and `self.span`
is the plots extent.

I suggest not to go into the details of the algorithm. The description is not accurate enough, e.g. sn is the number of arrows must be sn is the number of arrows in X-direction, but at least 10. But these details do not help the user. It should be sufficient to state that the arrows are autoscaled to a reasonable size. Knowledge of the algorithm does not make a difference, because you cannot tune any of the parameters. If the autoscaling does not yield a reasonable result, the user has to manually define a scale.

One important aspect to mention is that setting scale_units without setting scale does not have any effect because the scale units only differ by a constant factor and that is rescaled through autoscaling.

@Kaustbh
Copy link
Contributor Author
Kaustbh commented Oct 12, 2024

Hi, is the length we are getting from length = a * (widthu_per_lenu / (self.scale * self.width)) is in pixels?

x_pos = 0
y_pos = 0
u = 1
v = 1
 
fig, ax = plt.subplots()
ax.quiver(x_pos, y_pos, u, v,
         scale = 5,scale_units='width')
 

ax.set_xlim(-5, 5)
ax.set_ylim(-7, 10)

plt.grid(True)
plt.show() 

for the above code, I am getting the length as [37.71236166], and the plot I am getting is this,

plot1

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?
am I doing something wrong? please guide me.

@timhoffm
Copy link
Member

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 quiver(..., width=...) and check that this scales length.

@melissawm melissawm added the Documentation: API files in lib/ and doc/api label Oct 15, 2024
@github-actions github-actions bot removed the Documentation: API files in lib/ and doc/api label Oct 18, 2024
@Kaustbh
Copy link
Contributor Author
Kaustbh commented Oct 18, 2024

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 :math: renderer. I was not able to render the documentation.

Please tell me if it is fine or need more changes.

Comment on lines 138 to 141
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.
Copy link
Member

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

Suggested change
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.
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)

Comment on lines 143 to 145
- '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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- '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.

Comment on lines 146 to 147
"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."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"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.

@Kaustbh
Copy link
Contributor Author
Kaustbh commented Oct 18, 2024

is it correct for the case of 'x' or 'y'? , I have modified it like this,

- 'x' or 'y': The arrow length is scaled relative to the x or y axis units.
       For example, if *scale_units* is 'x' and *scale* is 1.0, the vector will be
       x-axis units long.
       The generalize formula is as follows:
            length in x direction = $\frac{u}{\mathrm{scale}}
            length in y direction = $\frac{v}{\mathrm{scale}}

@timhoffm
Copy link
Member

is it correct for the case of 'x' or 'y'?

I'm unclear what "it" is. Anyway, #28863 (comment) cover 'x' and 'y' cases as well.

Copy link
Member
@timhoffm timhoffm left a 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.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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

@Kaustbh
Copy link
Contributor Author
Kaustbh commented Oct 25, 2024

Hi, are there any changes to be made?

@timhoffm
Copy link
Member

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.

@timhoffm timhoffm merged commit 64d984a into matplotlib:main Oct 26, 2024
41 of 42 checks passed
@timhoffm timhoffm added this to the v3.10.0 milestone Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0