Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
09dd9aa tof155550CompareSorry 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'tangle 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 theRectangle 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 forEllipse 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 theEllipse patch - sorry I read too quickly!
Is it worth having aget_corners property as public API for theEllipse patch? Maybe there could a private API_get_corners for both (Rectangle andEllipse) andRectangle patch could have the publicget_corners as a convenience property?
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.
Uh oh!
There was an error while loading.Please reload this page.
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
pytestpasses).flake8-docstringsand 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).