-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add corner coordinate helper methods to Ellipse/Rectangle #21977
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.
It would be good to add a test with rotation.
2251bd9
to
5930b57
Compare
09dd9aa
to
f155550
Compare
Sorry for the messy PR... I've dropped edge_centers (I'll add them as private API in the widget PR), and should have resolved all the other comments. |
lib/matplotlib/patches.py
Outdated
def get_corners(self): | ||
""" | ||
Return the corners of the ellipse bounding box, moving anti-clockwise | ||
from the lower left. |
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.
Is it really lower left? Can't angle
be over 90?
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.
Yes, it's lower left in the de-rotated frame. I'm not sure what the best way to phrase this is - perhaps just "from the lower left before rotation is applied."?
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.
Would it better and clear enough to use "anchor point" (xy
), as this term is used in the Rectangle
patch docstring?
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 that works for Ellipse
though, since the "anchor point" is the center.
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.
Indeed, it isn't sensible for the Ellipse
patch - sorry I read too quickly!
Is it worth having a get_corners
property as public API for the Ellipse
patch? Maybe there could a private API _get_corners
for both (Rectangle
and Ellipse
) and Rectangle
patch could have the public get_corners
as a convenience property?
f155550
to
195b243
Compare
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.
#21977 (comment) could be improved but this is already good.
195b243
to
2f334c8
Compare
PR Summary
This is pulled out of #21945 because it's a standalone feature, and I think it's worth the scrutiny of a separate PR.
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).