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

Update docs for FancyArrowPatch & Annotation to make it clear that ShrinkA/B parameters are in points and not fractional. #27947

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

thehappycheese
Copy link
Contributor

PR summary

closes #27941

Updates documentation in two places to make it more clear that FancyArrowPatch uses point units for the shrinkA & shrinkB parameters.

This was particularly unclear when accessing FancyArrowPatch via Axes.annotate() because there is an alternate "simple" way to specify an arrow that uses shink as a fraction rather than as points.
Therefore I have also proposed changes to the Annotation class in the text module.

Also please summarize the changes in the title, for example "Raise ValueError on
non-numeric input to set_xlim" and avoid non-descriptive titles such as "Addresses
issue #8576".

PR checklist

Copy link

@github-actions github-actions bot left a 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.

@thehappycheese
Copy link
Contributor Author

It could be argued that the second commit/change to Annotation is not needed since the unit is specified in the default value.

However I personally failed to spot that because Annotation also accepts a simple arrow format where shrink is fractional. When switching from simple to fancy I could not spot the problem. Therefore I still included it in this PR.

I would be happy to re-submit this PR without the change to Annotation if the maintainers feel it isnt needed.

I am pretty sure the change to FancyArrowPatch is needed though.

@timhoffm timhoffm added the Documentation: API files in lib/ and doc/api label Mar 19, 2024
@timhoffm timhoffm merged commit c74365f into matplotlib:main Mar 19, 2024
45 of 46 checks passed
@timhoffm
Copy link
Member

Thanks @thehappycheese, and congratulations on your first PR merged into Matplotlib! We hope to hear from you again.

@timhoffm timhoffm added this to the v3.9.0 milestone Mar 19, 2024
@QuLogic QuLogic changed the title Update docs for FancyArrowPatch & Annotation to make it clear that ShinkA/B parameters are in points and not fractional. Update docs for FancyArrowPatch & Annotation to make it clear that ShrinkA/B parameters are in points and not fractional. Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation: API files in lib/ and doc/api topic: text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: ShrinkA and ShrinkB are ignored in ax.annotate(arrowprops=...)
2 participants