Simplify callable(self._contains) checks#13057
Conversation
timhoffm
left a comment
timhoffm
left a comment
There was a problem hiding this comment.
Not quite sure if the change is worth the effort. But if so, it should be an explicit ˋis not Noneˋ check.
| implementation of the respective artist, but may provide | ||
| additional information. | ||
| """ | ||
| if not callable(picker): |
There was a problem hiding this comment.
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 general PEP8. |
twmr
force-pushed
the
simplifycallable
branch
from
cfb6c29 to
ac470d9
Compare
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 self._contains is not None.
twmr
force-pushed
the
simplifycallable
branch
from
ac470d9 to
06081dc
Compare
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.x Backport PR #13057 on branch v3.1.x (Simplify callable(self._contains) checks)
4 participants
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