Deprecate setting a Collection/Patch's pickradius via set_picker. #19026

anntzer · Nov 27, 2020

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_radius was
likely wrong: after calling set_picker(True), _picker would be a
bool and therefore a Number, so the fallback cases to the patch
linewidth did not occur.

PR Summary

PR Checklist

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (run flake8 on changed files to check).
New features are documented, with examples if plot related.
Documentation is sphinx and numpydoc compliant (the docs should build without error).
Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

QuLogic · Nov 28, 2020

lib/mpl_toolkits/tests/test_axes_grid1.py

@@ -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)


Maybe you want to keep one of these for the test coverage?

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_radius` was likely wrong: after calling `set_picker(True)`, `_picker` would be a bool and therefore a `Number`, so the fallback cases to the patch linewidth did not occur.

QuLogic · Dec 8, 2020

doc/api/next_api_changes/deprecations/19026-AL.rst

+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;


Suggested change

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;

QuLogic · Dec 10, 2020

lib/matplotlib/collections.py

+                "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)


Should you then set p to True since you've removed the type checks later?

and probably make the same change in lines.py as well?

QuLogic · Dec 10, 2020

lib/matplotlib/patches.py

+                "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)


tacaswell

👍 in principle, but concerned about Patch.

tacaswell · Dec 19, 2020

lib/matplotlib/collections.py

+                "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)


and probably make the same change in lines.py as well?

tacaswell · Dec 19, 2020

lib/matplotlib/artist.py

-              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


Suggested change

- *deprecated*: For `.Line2D` and `.Collection`, *picker* can also

- *deprecated*: For `.Line2D`, `.Patch` and `.Collection`, *picker* can also

tacaswell · Dec 19, 2020

lib/matplotlib/artist.py

+            - *deprecated*: For `.Line2D` and `.Collection`, *picker* can also
+              be a float that sets the tolerance for checking whether an event
+              occurred "on" the artist; this is deprecated.  Use
+              `.Line2D.set_pickradius`/`.Collection.set_pickradius` instead.


Suggested change

`.Line2D.set_pickradius`/`.Collection.set_pickradius` instead.

`.Line2D.set_pickradius`/`.Collection.set_pickradius`/`.Patch.set_pickradius`

instead.

tacaswell · Dec 19, 2020

lib/matplotlib/patches.py

+            self.set_pickradius(p)
+        self._picker = p
+
+    def set_pickradius(self, d):


Did Patch not have set_pickradius before? I'm concerned about deprecating this and adding the replacement API in the same version.

anntzer · Dec 19, 2020

We may want to resolve #19039 first.

anntzer · Jan 21, 2021

Superseded per #19039 (comment).

anntzer added this to the v3.4.0 milestone Nov 27, 2020

anntzer force-pushed the collection-pick-radius branch 2 times, most recently from 137a3b5 to 9766c90 Compare November 27, 2020 11:25

anntzer changed the title ~~Deprecate setting a Collection's pickradius via set_picker.~~ Deprecate setting a Collection/Patch's pickradius via set_picker. Nov 27, 2020

anntzer force-pushed the collection-pick-radius branch 2 times, most recently from ebb9240 to 6d8f305 Compare November 27, 2020 14:55

timhoffm approved these changes Nov 27, 2020

View reviewed changes

QuLogic reviewed Nov 28, 2020

View reviewed changes

anntzer force-pushed the collection-pick-radius branch from 6d8f305 to 4cf5368 Compare November 28, 2020 12:31

anntzer mentioned this pull request Nov 28, 2020

Should set_pickradius() implicitly call set_picker(True)? #19039

Open

anntzer force-pushed the collection-pick-radius branch from 4cf5368 to ebd6803 Compare November 28, 2020 13:27

QuLogic reviewed Dec 10, 2020

View reviewed changes

tacaswell reviewed Dec 19, 2020

View reviewed changes

QuLogic added the status: needs comment/discussion needs consensus on next step label Jan 5, 2021

anntzer closed this Jan 21, 2021

anntzer deleted the collection-pick-radius branch January 21, 2021 20:31

QuLogic removed this from the v3.4.0 milestone Mar 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Deprecate setting a Collection/Patch's pickradius via set_picker. #19026

Deprecate setting a Collection/Patch's pickradius via set_picker. #19026

Uh oh!

anntzer commented Nov 27, 2020 •

edited

Loading

Uh oh!

QuLogic Nov 28, 2020

Uh oh!

anntzer Nov 28, 2020

Uh oh!

QuLogic Dec 8, 2020

Uh oh!

QuLogic Dec 10, 2020

Uh oh!

tacaswell Dec 19, 2020

Uh oh!

QuLogic Dec 10, 2020

Uh oh!

tacaswell left a comment

Uh oh!

tacaswell Dec 19, 2020

Uh oh!

tacaswell Dec 19, 2020

Uh oh!

tacaswell Dec 19, 2020

Uh oh!

tacaswell Dec 19, 2020

Uh oh!

anntzer commented Dec 19, 2020

Uh oh!

anntzer commented Jan 21, 2021

Uh oh!

Uh oh!

	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;

	- deprecated: For `.Line2D` and `.Collection`, picker can also
	- deprecated: For `.Line2D`, `.Patch` and `.Collection`, picker can also

	`.Line2D.set_pickradius`/`.Collection.set_pickradius` instead.
	`.Line2D.set_pickradius`/`.Collection.set_pickradius`/`.Patch.set_pickradius`
	instead.

Search code, repositories, users, issues, pull requests...

Uh oh!

Deprecate setting a Collection/Patch's pickradius via set_picker. #19026

Deprecate setting a Collection/Patch's pickradius via set_picker. #19026

Uh oh!

Conversation

anntzer commented Nov 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tacaswell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anntzer commented Dec 19, 2020

Uh oh!

anntzer commented Jan 21, 2021

Uh oh!

Uh oh!

anntzer commented Nov 27, 2020 •

edited

Loading