Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Deprecate setting a Collection/Patch's pickradius via set_picker.#19026
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
137a3b5 to9766c90Compareebb9240 to6d8f305Compare| @@ -409,8 +409,9 @@ def test_picking_callbacks_overlap(big_on_axes, small_on_axes, click_on): | |||
| # In each case we expect that both rectangles are picked if we click on the | |||
| # small one and only the big one is picked if we click on the big one. | |||
| # Also tests picking on normal axes ("gca") as a control. | |||
| big = plt.Rectangle((0.25, 0.25), 0.5, 0.5, picker=5) | |||
| small = plt.Rectangle((0.4, 0.4), 0.2, 0.2, facecolor="r", picker=5) | |||
| big = plt.Rectangle((0.25, 0.25), 0.5, 0.5, picker=True, pickradius=5) | |||
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.
Maybe you want to keep one of these for the test coverage?
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.
sure
6d8f305 to4cf5368CompareThis is the same deprecation as the one introduced for Line2D in 3.3.(The exact deprecation strategy used here may already break convolutedcall sequences on Collections: calling`coll.set_pickradius(3); coll.set_picker(5); coll.set_picker(False); coll.set_picker(True)`will leave the pickradius at 5 instead of 3 as was the case before, butI'm not too worried about that (if anything the new behavior is moresensical...).)On Patches, note that the old implementation of `_process_radius` waslikely wrong: after calling `set_picker(True)`, `_picker` would be abool and therefore a `Number`, so the fallback cases to the patchlinewidth did not occur.
4cf5368 toebd6803Compare| Setting pickradius on Collections and Patches via ``set_picker`` | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| Setting the pickradius (i.e. the tolerance for pick events and containment | ||
| checks)of a `.Collection` or a `.Patch` via `.Artist.set_picker` is deprecated; |
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.
| checks)of a `.Collection` or a `.Patch` via `.Artist.set_picker` is deprecated; | |
| checks)of a `.Collection` or a `.Patch` via `.Artist.set_picker` is deprecated; |
| "3.4", message="Setting the collections's pick radius via " | ||
| "set_picker is deprecated since %(since)s and will be removed " | ||
| "%(removal)s; use set_pickradius instead.") | ||
| self.set_pickradius(p) |
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.
Should you then setp toTrue since you've removed the type checks later?
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.
and probably make the same change inlines.py as well?
| "3.4", message="Setting the collections's pick radius via " | ||
| "set_picker is deprecated since %(since)s and will be removed " | ||
| "%(removal)s; use set_pickradius instead.") | ||
| self.set_pickradius(p) |
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.
Ditto.
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.
👍 in principle, but concerned about Patch.
| "3.4", message="Setting the collections's pick radius via " | ||
| "set_picker is deprecated since %(since)s and will be removed " | ||
| "%(removal)s; use set_pickradius instead.") | ||
| self.set_pickradius(p) |
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.
and probably make the same change inlines.py as well?
| that sets the tolerance for checking whether an event occurred | ||
| "on" the line; this is deprecated. Use `.Line2D.set_pickradius` | ||
| instead. | ||
| - *deprecated*: For `.Line2D` and `.Collection`, *picker* can also |
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.
| -*deprecated*:For`.Line2D`and`.Collection`,*picker*canalso | |
| -*deprecated*:For`.Line2D`,`.Patch`and`.Collection`,*picker*canalso |
| - *deprecated*: For `.Line2D`and `.Collection`, *picker* can also | ||
| be a floatthat sets the tolerance for checking whether an event | ||
| occurred"on" theartist; this is deprecated. Use | ||
| `.Line2D.set_pickradius`/`.Collection.set_pickradius`instead. |
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.
| `.Line2D.set_pickradius`/`.Collection.set_pickradius`instead. | |
| `.Line2D.set_pickradius`/`.Collection.set_pickradius`/`.Patch.set_pickradius` | |
| instead. |
| self.set_pickradius(p) | ||
| self._picker = p | ||
| def set_pickradius(self, d): |
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.
DidPatch not haveset_pickradius before? I'm concerned about deprecating this and adding the replacement API in the same version.
We may want to resolve#19039 first. |
Superseded per#19039 (comment). |
Uh oh!
There was an error while loading.Please reload this page.
This is the same deprecation as the one introduced for Line2D in 3.3.
(The exact deprecation strategy used here may already break convoluted
call sequences on Collections: calling
coll.set_pickradius(3); coll.set_picker(5); coll.set_picker(False); coll.set_picker(True)will leave the pickradius at 5 instead of 3 as was the case before, but
I'm not too worried about that (if anything the new behavior is more
sensical...).)
On Patches, note that the old implementation of
_process_radiuswaslikely wrong: after calling
set_picker(True),_pickerwould be abool and therefore a
Number, so the fallback cases to the patchlinewidth did not occur.
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).