Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Fix OffsetBox custom picker#30096
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
Are you able to bring one of the tests from mplcursors over to get coverage of the new line? |
Possibly, but they all use |
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.
Looks correct, and definitely fixes mplcursors' test suite. A test would be nice indeed.
Also the logic looks a bit inverted, I'd write this asif mouseevent.name == "scroll_event": return False, {} else: return artist.contains(mouseevent), not that it really matters.
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.
Optionally, take suggestions by@anntzer
As with the custom picker, `Artist.contains` returns a boolean and adictionary in a tuple. This non-empty tuple is always true, so thecustom picker would always return True for any non-scroll event. Itwould also lose the related dictionary.
948a26f intomatplotlib:mainUh oh!
There was an error while loading.Please reload this page.
PR summary
As with the custom picker,
Artist.containsreturns a boolean and a dictionary in a tuple. This non-empty tuple is always true, so the custom picker would always return True for any non-scroll event. It would also lose the related dictionary.This broke
mplcursorstests.PR checklist