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

TransformedBbox.contains has less-than-optimal semantics #12057

Closed
anntzer opened this issue Sep 7, 2018 · 2 comments · Fixed by #25902
Closed

TransformedBbox.contains has less-than-optimal semantics #12057

anntzer opened this issue Sep 7, 2018 · 2 comments · Fixed by #25902
Milestone

Comments

@anntzer
Copy link
Contributor

anntzer commented Sep 7, 2018

TransformedBbox inherits the contains() method from Bbox, which only checks separately that x is between xmin and xmax, and that y is between ymin and ymax. This means that a TransformedBbox with 45° rotation will incorrectly believe to contain() points that are "in the non-covered corners" (xref #12052).

It's not clear whether we actually need TransformedBbox.contains to work for non-axis-aligned Bboxes (perhaps Patches are better structures for that), but the current behavior seems to be just a good way to shoot oneself in the foot. Either we should fix the implementation of contains() (I think(?) we can just apply the inverse-transform to the point and check whether it is in the original, untransformed Bbox), or at least TransformedBbox.contains should raise some exception when called and the Bbox is not axis-aligned.

@github-actions
Copy link

This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!

@github-actions github-actions bot added the status: inactive Marked by the “Stale” Github Action label May 16, 2023
@anntzer
Copy link
Contributor Author

anntzer commented May 16, 2023

Investigating whether we can just inverse-transform the point before checking containment in the untransformed bbox still seems reasonable to me.

@anntzer anntzer added keep Items to be ignored by the “Stale” Github Action and removed status: inactive Marked by the “Stale” Github Action labels May 16, 2023
@QuLogic QuLogic removed the keep Items to be ignored by the “Stale” Github Action label May 22, 2023
@QuLogic QuLogic added this to the v3.7.2 milestone May 22, 2023
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 a pull request may close this issue.

2 participants