Repository navigation
[MNT]: Deprecate onselect/onmove_callback of selectors #21929
Description
Summary
If I remember correctly, at some point, @anntzer suggested to deprecate onselect argument for selectors. Now, that I understand better what he meant at the time, I think it makes a lot of sense, because the behaviour of the "builtin" selector callback is contentious and inconsistent between selectors.
Proposed fix
The alternative would be something along the lines of:
import matplotlib.pyplot as plt
from matplotlib.widgets import RectangleSelector
import functools
_, ax = plt.subplots()
ax.plot([1, 2, 3])
def dummy_callback(*args):
pass
def onselect_callback(event, selector):
print('## onselect callback ##')
if not selector.ignore(event):
print(' ', selector.extents)
return
print(' event ignored')
def onmove_callback(event, selector):
print('## onmove callback ##')
if not selector.ignore(event) and selector._eventpress:
print(' ', selector.extents)
return
print(' event ignored')
tool = RectangleSelector(ax, dummy_callback, interactive=True,
drag_from_anywhere=True,)
tool.extents = (1.2, 2.0, 1.0, 1.6)
# alternative to onselect
rectangle_onselect_callback = functools.partial(onselect_callback, selector=tool)
tool.connect_event('button_release_event', rectangle_onselect_callback)
# alternative to onmove_callback (SpanSelector only)
rectangle_onmove_callback = functools.partial(onmove_callback, selector=tool)
tool.connect_event('motion_notify_event', rectangle_onmove_callback)In my opinion, the reasons to deprecate are:
- provide consistent API between selectors
- leave the user define what is the expected behaviour - anyway, I don't think the current API is obvious to use
- fix Widgets: RectangleSelector in Mpl 2.1.0 does not behave as described #9608 (see Fix for #9608: RectangleSelector now reports mouse button press and release events correctly #20611 for more discussion)
- simplify maintenance
Proposed fix:
- Deprecate the
onselect/onmovearguments of the selectors, as per example above - improve the API to ignore events (the example above uses a private API to reproduce the
onmove_callbackof theSpanSelector) - update example, improve documentation on the use of callback registry.
Activity
ericpre commentedon Dec 11, 2021
cc @dstansby, @dhomeier
dstansby commentedon Dec 11, 2021
I think at least making the
onselectargument a keyword argument, and defaulting to a no-op functions would be great, since it's often common to just want a selector without attaching a callback to it.As for deprecating, I'm not too sure what the motivation is?
anntzer commentedon Dec 12, 2021
I think you may be referring to #20603, specifically #20603 (comment)? There, the point was to deprecate the
on*methods, not the kwargs (which I think are fine, although defaulting to noop seems good too?).ericpre commentedon Dec 12, 2021
Yes, this is what I was referring to and indeed this is different discussion - sorry, I should have checked it up before mentioning it here!
I have list some reasons above:
To clarify the inconsistency with signature of the arguments (
onselectandonmove_callback), here are the correspoding docstrings:SpanSelector:RectangleSelector:LassoSelector:PolygonSelector:QuLogic commentedon Sep 20, 2024
#26000 only made these optional; do we want to do the deprecation as well?