Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
artist.set_picker(False) no longer keeps artists pickable#19031
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
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
The documentation for `set_picker` currently states that `None` disablespicking, `True` enables picking, and a custom callable implements, well,custom picking behavior. It turns out that `False` *also* enablespicking, because the relevant check is implemented in `Artist.pickable`,which strictly checks `picker is not None` (and also checks that theartist has been added to a figure). This can indeed be verified with```pythonfrom pylab import *plot([1, 2], picker=False); gcf().canvas.mpl_connect("pick_event", print)```and clicking on the line. Strictly speaking, the docs state nothingabout `False`, so the behavior does not violate the docs, but that'sclearly non-optimal behavior, which I'm proposing to change here.The deprecated call of |
Fair point. Do you think this warrants a deprecation period? In which case the whole PR should perhaps be turned into a deprecation of the old behavior rather than fully modifying it as it does right now. |
Is there a practical difference between a pick or radius zero and turning it off? |
I think so? For a filled rectangle even with pickradius=0 you should be able to pick it anywhere in the rectangle. |
timhoffm commentedDec 15, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
|
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.
Restarted appveyor.
Anyone can merge on CI green.
I'm not sure if the discussion is finished? |
We may want to resolve#19039 first, indeed. |
Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it. |
The documentation for
set_pickercurrently states thatNonedisablespicking,
Trueenables picking, and a custom callable implements, well,custom picking behavior. It turns out that
Falsealso enablespicking, because the relevant check is implemented in
Artist.pickable,which strictly checks
picker is not None(and also checks that theartist has been added to a figure). This can indeed be verified with
and clicking on the line. Strictly speaking, the docs state nothing
about
False, so the behavior does not violate the docs, but that'sclearly non-optimal behavior, which I'm proposing to change here.
PR Summary
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).