Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Simplify callable(self._contains) checks#13057
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.
Not quite sure if the change is worth the effort. But if so, it should be an explicit ˋis not Noneˋ check.
| @@ -430,6 +430,8 @@ def contains(artist: Artist, event: MouseEvent) -> bool, dict | |||
| implementation of the respective artist, but may provide | |||
| additional information. | |||
| """ | |||
| if not callable(picker): | |||
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 think it‘s reasonable to keep this check. And not just fail when trying to call _contains
I'm also not really sure if it is worth the effort.@rmorshea do you remember why you had to change the @timhoffm is it documented somewhere that the explicit |
None-check handling is not documented within matplotlib. However, it's generalPEP8. |
Artist._contains can either be None (not-set) or a callable (seedocstring of Artist.set_contains). Therefore, thecallable(self._contains) checks can be simplified toself._contains is not None.
Thx. I've added "is not None" to all self._contains checks. |
This is a small change, bit still to good. |
Thanks@Thisch ! |
Something went wrong ... Please have a look at my logs. |
…057-on-v3.1.xBackport PR#13057 on branch v3.1.x (Simplify callable(self._contains) checks)
Artist._contains can either be None (not-set) or a callable (see
docstring of Artist.set_contains). Therefore, the
callable(self._contains) checks can be simplified to
bool(self._contains).
PR Summary
I had a look at the traitlets PR (#4762, still has unresolved merge conflicts) and noticed some parts that could be moved to its own PR.
PR Checklist