Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
In Artist.contains, check that moussevents occurred on the right canvas.#25546
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
Uh oh!
There was an error while loading.Please reload this page.
| return (getattr(event, "canvas", None) is not None and self.figure is not None | ||
| and event.canvas is not self.figure.canvas) |
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.
Rather than guarding against theNone is None case, could you do something like this instead?
| return (getattr(event,"canvas",None)isnotNoneandself.figureisnotNone | |
| andevent.canvasisnotself.figure.canvas) | |
| return (getattr(event,"canvas",object())isnotgetattr(self.figure,"canvas",object()) |
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 so? That would incorrectly return "different" if event.canvas = None and self.figure != None and self.figure.canvas = anything, I believe?
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.
Ahh, yes. My confusion seems to agree with the other confusion on the "maybe"/"for sure" same canvas comments above.
I wonder if there is a way to clear up that maybe_same_canvas by guaranteeing these attributes are set elsewhere in another PR.
Uh oh!
There was an error while loading.Please reload this page.
The old _default_contains check was geared towards custom containscheckers (set via set_contains), which have been removed. Get rid ofthat and instead, add a check for canvas identity which is more useful.
The old _default_contains check was geared towards custom contains checkers (set via set_contains), which have been removed. Get rid of that and instead, add a check for canvas identity which is more useful.
Closes#6324.
PR Summary
PR Checklist
Documentation and Tests
pytestpasses)Release Notes
.. versionadded::directive in the docstring and documented indoc/users/next_whats_new/.. versionchanged::directive in the docstring and documented indoc/api/next_api_changes/next_whats_new/README.rstornext_api_changes/README.rst