Skip to content
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

Fix quiver key plot when angles='xy' and/or scale_units='xy' #27044

Merged
merged 1 commit into from Jan 24, 2024

Conversation

stevezhang1999
Copy link
Contributor

PR summary

Closes #26316 #26748 #27008

Fix incorrect behavior for quiver key. When angles='xy' and/or scale_units='xy', incorrect results will be produced for polar and aitoff.

Problem:

This happens because ax.quiverkey() reuses quiver._make_verts() to produce the arrow. During the call, quiver._angles_lengths() will be called when angles='xy' and/or scale_units='xy'. This methods will calculate the results using [X, Y] input from quiver() anyway, therefore, causing the result quiverkey() be broadcasted to wrong demension (instead of 1). This also applies to Cartesian - even if the result looks correct, multiple arrows were plotted at the same place.

Fix:

  1. ax.quiverkey() will call quiver._make_verts(..., angles) only with angles='uv'. This is because we are using degrees to describe the angle for quiver key, not two points.
  2. quiver._angles_lengths() now explicitly requires inputing [X, Y], rather than using those from ax.quiver() call.
  3. A new baseline image for test_quiver.test_quiver_key_xy - the old one looks different than how it is specified in the doc [Bug]: quiverkey behavior with quiver(scale_units="xy") #27008

PR checklist

@kamunwasam

This comment was marked as off-topic.

@dstansby dstansby added this to the v3.8.3 milestone Dec 28, 2023
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

This looks good to me, and I checked it fixed the linked issues 👍 Could you squash the commits together so the PR cleanliness test passes? I think there's some issue with commits/test images that squashing should fix.

change doc for angle to horizonal axis

correct test_quiver_key_xy image

fix Quiver._angles_lengths

add tests and fix original test image
@stevezhang1999
Copy link
Contributor Author

This looks good to me, and I checked it fixed the linked issues 👍 Could you squash the commits together so the PR cleanliness test passes? I think there's some issue with commits/test images that squashing should fix.

Thanks for your kind comment and help! I missed this message as I have been away from mpl for a while. Please let me know if I can help.

@ksunden ksunden merged commit 661d32a into matplotlib:main Jan 24, 2024
39 of 42 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jan 24, 2024
QuLogic added a commit that referenced this pull request Jan 25, 2024
…044-on-v3.8.x

Backport PR #27044 on branch v3.8.x (Fix quiver key plot when angles='xy' and/or scale_units='xy')
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: quiverkey shows multiple arrows under geographical projection and angle='xy'
4 participants