From 5731ff123673505d91b17412f7a89822bc53983f Mon Sep 17 00:00:00 2001 From: Antony Lee Date: Fri, 25 Apr 2025 18:48:44 +0200 Subject: [PATCH] Fix AxesWidgets on inset_axes that are outside their parent. When axes are overlapping, LocationEvent.inaxes can point not to the axes we care about, but to another one. Widgets currently recompute location event axes coordinates relative to the axes to which the widget is assigned, but this recomputation code was previously brittle wrt. events that are outside the axes they wrongly believe they belong to (so x/ydata is None), even though they are indeed within the axes they actually belong to. This can occur when a widget is associated with an "inset_axes" that's actually outside the parent axes. A practical case is given by ```python from pylab import * ax = figure(layout="constrained").add_subplot() ax1 = ax.inset_axes([0, 1, 1, .25], sharex=ax) ss = mpl.widgets.SpanSelector(ax1, print, "horizontal") show() # try to trigger the spanselector ``` which would raise an exception prior to this patch. Improve the recomputation logic by fully reparenting the event passed to the widget to the correct parent axes from the onset. --- lib/matplotlib/tests/test_widgets.py | 43 +++++----- lib/matplotlib/widgets.py | 118 ++++++++++++++++++--------- 2 files changed, 101 insertions(+), 60 deletions(-) diff --git a/lib/matplotlib/tests/test_widgets.py b/lib/matplotlib/tests/test_widgets.py index 808863fd6a94..2b3fcc2274ab 100644 --- a/lib/matplotlib/tests/test_widgets.py +++ b/lib/matplotlib/tests/test_widgets.py @@ -623,27 +623,30 @@ def test_rectangle_selector_ignore_outside(ax, ignore_event_outside): ('horizontal', False, dict(interactive=True)), ]) def test_span_selector(ax, orientation, onmove_callback, kwargs): - onselect = mock.Mock(spec=noop, return_value=None) - onmove = mock.Mock(spec=noop, return_value=None) - if onmove_callback: - kwargs['onmove_callback'] = onmove - - # While at it, also test that span selectors work in the presence of twin axes on - # top of the axes that contain the selector. Note that we need to unforce the axes - # aspect here, otherwise the twin axes forces the original axes' limits (to respect - # aspect=1) which makes some of the values below go out of bounds. + # Also test that span selectors work in the presence of twin axes or for + # outside-inset axes on top of the axes that contain the selector. Note + # that we need to unforce the axes aspect here, otherwise the twin axes + # forces the original axes' limits (to respect aspect=1) which makes some + # of the values below go out of bounds. ax.set_aspect("auto") - tax = ax.twinx() - - tool = widgets.SpanSelector(ax, onselect, orientation, **kwargs) - do_event(tool, 'press', xdata=100, ydata=100, button=1) - # move outside of axis - do_event(tool, 'onmove', xdata=199, ydata=199, button=1) - do_event(tool, 'release', xdata=250, ydata=250, button=1) - - onselect.assert_called_once_with(100, 199) - if onmove_callback: - onmove.assert_called_once_with(100, 199) + ax.twinx() + child = ax.inset_axes([0, 1, 1, .05], xlim=(0, 200), ylim=(0, 200)) + + for target in [ax, child]: + onselect = mock.Mock(spec=noop, return_value=None) + onmove = mock.Mock(spec=noop, return_value=None) + if onmove_callback: + kwargs['onmove_callback'] = onmove + + tool = widgets.SpanSelector(target, onselect, orientation, **kwargs) + do_event(tool, 'press', xdata=100, ydata=100, button=1) + # move outside of axis + do_event(tool, 'onmove', xdata=199, ydata=199, button=1) + do_event(tool, 'release', xdata=250, ydata=250, button=1) + + onselect.assert_called_once_with(100, 199) + if onmove_callback: + onmove.assert_called_once_with(100, 199) @pytest.mark.parametrize('interactive', [True, False]) diff --git a/lib/matplotlib/widgets.py b/lib/matplotlib/widgets.py index 6b196571814d..1c4cdd1cc419 100644 --- a/lib/matplotlib/widgets.py +++ b/lib/matplotlib/widgets.py @@ -11,6 +11,7 @@ from contextlib import ExitStack import copy +import functools import itertools from numbers import Integral, Number @@ -134,15 +135,31 @@ def disconnect_events(self): for c in self._cids: self.canvas.mpl_disconnect(c) - def _get_data_coords(self, event): - """Return *event*'s data coordinates in this widget's Axes.""" - # This method handles the possibility that event.inaxes != self.ax (which may - # occur if multiple Axes are overlaid), in which case event.xdata/.ydata will - # be wrong. Note that we still special-case the common case where - # event.inaxes == self.ax and avoid re-running the inverse data transform, - # because that can introduce floating point errors for synthetic events. - return ((event.xdata, event.ydata) if event.inaxes is self.ax - else self.ax.transData.inverted().transform((event.x, event.y))) + +def _call_with_reparented_event(func): + """ + Event callback decorator ensuring that the callback is called with an event + that has been reparented to the widget's axes. + """ + # This decorator handles the possibility that event.inaxes != self.ax + # (e.g. if multiple Axes are overlaid), in which case event.xdata/.ydata + # will be wrong. Note that we still special-case the common case where + # event.inaxes == self.ax and avoid re-running the inverse data transform, + # because that can introduce floating point errors for synthetic events. + @functools.wraps(func) + def wrapper(self, event): + if event.inaxes is not self.ax: + event = copy.copy(event) + event.guiEvent = None + event.inaxes = self.ax + try: + event.xdata, event.ydata = ( + self.ax.transData.inverted().transform((event.x, event.y))) + except ValueError: # cf LocationEvent._set_inaxes. + event.xdata = event.ydata = None + return func(self, event) + + return wrapper class Button(AxesWidget): @@ -209,12 +226,14 @@ def __init__(self, ax, label, image=None, self.color = color self.hovercolor = hovercolor + @_call_with_reparented_event def _click(self, event): if not self.eventson or self.ignore(event) or not self.ax.contains(event)[0]: return if event.canvas.mouse_grabber != self.ax: event.canvas.grab_mouse(self.ax) + @_call_with_reparented_event def _release(self, event): if self.ignore(event) or event.canvas.mouse_grabber != self.ax: return @@ -222,6 +241,7 @@ def _release(self, event): if self.eventson and self.ax.contains(event)[0]: self._observers.process('clicked', event) + @_call_with_reparented_event def _motion(self, event): if self.ignore(event): return @@ -520,6 +540,7 @@ def _value_in_bounds(self, val): val = self.slidermax.val return val + @_call_with_reparented_event def _update(self, event): """Update the slider position.""" if self.ignore(event) or event.button != 1: @@ -538,9 +559,8 @@ def _update(self, event): event.canvas.release_mouse(self.ax) return - xdata, ydata = self._get_data_coords(event) val = self._value_in_bounds( - xdata if self.orientation == 'horizontal' else ydata) + event.xdata if self.orientation == 'horizontal' else event.ydata) if val not in [None, self.val]: self.set_val(val) @@ -853,6 +873,7 @@ def _update_val_from_pos(self, pos): else: self._active_handle.set_xdata([val]) + @_call_with_reparented_event def _update(self, event): """Update the slider position.""" if self.ignore(event) or event.button != 1: @@ -873,11 +894,10 @@ def _update(self, event): return # determine which handle was grabbed - xdata, ydata = self._get_data_coords(event) handle_index = np.argmin(np.abs( - [h.get_xdata()[0] - xdata for h in self._handles] + [h.get_xdata()[0] - event.xdata for h in self._handles] if self.orientation == "horizontal" else - [h.get_ydata()[0] - ydata for h in self._handles])) + [h.get_ydata()[0] - event.ydata for h in self._handles])) handle = self._handles[handle_index] # these checks ensure smooth behavior if the handles swap which one @@ -885,7 +905,8 @@ def _update(self, event): if handle is not self._active_handle: self._active_handle = handle - self._update_val_from_pos(xdata if self.orientation == "horizontal" else ydata) + self._update_val_from_pos( + event.xdata if self.orientation == "horizontal" else event.ydata) def _format(self, val): """Pretty-print *val*.""" @@ -1090,6 +1111,7 @@ def _clear(self, event): self._background = self.canvas.copy_from_bbox(self.ax.bbox) self.ax.draw_artist(self._checks) + @_call_with_reparented_event def _clicked(self, event): if self.ignore(event) or event.button != 1 or not self.ax.contains(event)[0]: return @@ -1396,6 +1418,7 @@ def _rendercursor(self): fig.canvas.draw() + @_call_with_reparented_event def _release(self, event): if self.ignore(event): return @@ -1403,6 +1426,7 @@ def _release(self, event): return event.canvas.release_mouse(self.ax) + @_call_with_reparented_event def _keypress(self, event): if self.ignore(event): return @@ -1485,6 +1509,7 @@ def stop_typing(self): # call it once we've already done our cleanup. self._observers.process('submit', self.text) + @_call_with_reparented_event def _click(self, event): if self.ignore(event): return @@ -1500,9 +1525,11 @@ def _click(self, event): self.cursor_index = self.text_disp._char_index_at(event.x) self._rendercursor() + @_call_with_reparented_event def _resize(self, event): self.stop_typing() + @_call_with_reparented_event def _motion(self, event): if self.ignore(event): return @@ -1668,6 +1695,7 @@ def _clear(self, event): self._background = self.canvas.copy_from_bbox(self.ax.bbox) self.ax.draw_artist(self._buttons) + @_call_with_reparented_event def _clicked(self, event): if self.ignore(event) or event.button != 1 or not self.ax.contains(event)[0]: return @@ -1924,6 +1952,7 @@ def clear(self, event): if self.useblit: self.background = self.canvas.copy_from_bbox(self.ax.bbox) + @_call_with_reparented_event def onmove(self, event): """Internal event handler to draw the cursor when the mouse moves.""" if self.ignore(event): @@ -1938,10 +1967,9 @@ def onmove(self, event): self.needclear = False return self.needclear = True - xdata, ydata = self._get_data_coords(event) - self.linev.set_xdata((xdata, xdata)) + self.linev.set_xdata((event.xdata, event.xdata)) self.linev.set_visible(self.visible and self.vertOn) - self.lineh.set_ydata((ydata, ydata)) + self.lineh.set_ydata((event.ydata, event.ydata)) self.lineh.set_visible(self.visible and self.horizOn) if not (self.visible and (self.vertOn or self.horizOn)): return @@ -2228,9 +2256,8 @@ def _get_data(self, event): """Get the xdata and ydata for event, with limits.""" if event.xdata is None: return None, None - xdata, ydata = self._get_data_coords(event) - xdata = np.clip(xdata, *self.ax.get_xbound()) - ydata = np.clip(ydata, *self.ax.get_ybound()) + xdata = np.clip(event.xdata, *self.ax.get_xbound()) + ydata = np.clip(event.ydata, *self.ax.get_ybound()) return xdata, ydata def _clean_event(self, event): @@ -2250,6 +2277,7 @@ def _clean_event(self, event): self._prev_event = event return event + @_call_with_reparented_event def press(self, event): """Button press handler and validator.""" if not self.ignore(event): @@ -2268,6 +2296,7 @@ def press(self, event): def _press(self, event): """Button press event handler.""" + @_call_with_reparented_event def release(self, event): """Button release event handler and validator.""" if not self.ignore(event) and self._eventpress: @@ -2283,6 +2312,7 @@ def release(self, event): def _release(self, event): """Button release event handler.""" + @_call_with_reparented_event def onmove(self, event): """Cursor move event handler and validator.""" if not self.ignore(event) and self._eventpress: @@ -2294,6 +2324,7 @@ def onmove(self, event): def _onmove(self, event): """Cursor move event handler.""" + @_call_with_reparented_event def on_scroll(self, event): """Mouse scroll event handler and validator.""" if not self.ignore(event): @@ -2302,6 +2333,7 @@ def on_scroll(self, event): def _on_scroll(self, event): """Mouse scroll event handler.""" + @_call_with_reparented_event def on_key_press(self, event): """Key press event handler and validator for all selection widgets.""" if self.active: @@ -2326,6 +2358,7 @@ def on_key_press(self, event): def _on_key_press(self, event): """Key press event handler - for widget-specific key press actions.""" + @_call_with_reparented_event def on_key_release(self, event): """Key release event handler and validator.""" if self.active: @@ -2647,8 +2680,7 @@ def _press(self, event): # Clear previous rectangle before drawing new rectangle. self.update() - xdata, ydata = self._get_data_coords(event) - v = xdata if self.direction == 'horizontal' else ydata + v = event.xdata if self.direction == 'horizontal' else event.ydata if self._active_handle is None and not self.ignore_event_outside: # when the press event outside the span, we initially set the @@ -2685,6 +2717,7 @@ def direction(self, direction): else: self._direction = direction + @_call_with_reparented_event def _release(self, event): """Button release event handler.""" self._set_cursor(False) @@ -2716,6 +2749,7 @@ def _release(self, event): return False + @_call_with_reparented_event def _hover(self, event): """Update the canvas cursor if it's over a handle.""" if self.ignore(event): @@ -2734,12 +2768,11 @@ def _hover(self, event): def _onmove(self, event): """Motion notify event handler.""" - xdata, ydata = self._get_data_coords(event) if self.direction == 'horizontal': - v = xdata + v = event.xdata vpress = self._eventpress.xdata else: - v = ydata + v = event.ydata vpress = self._eventpress.ydata # move existing span @@ -3242,9 +3275,8 @@ def _press(self, event): if (self._active_handle is None and not self.ignore_event_outside and self._allow_creation): - x, y = self._get_data_coords(event) self._visible = False - self.extents = x, x, y, y + self.extents = event.xdata, event.xdata, event.ydata, event.ydata self._visible = True else: self.set_visible(True) @@ -3255,6 +3287,7 @@ def _press(self, event): return False + @_call_with_reparented_event def _release(self, event): """Button release event handler.""" if not self._interactive: @@ -3321,7 +3354,7 @@ def _onmove(self, event): move = self._active_handle == 'C' resize = self._active_handle and not move - xdata, ydata = self._get_data_coords(event) + xdata, ydata = event.xdata, event.ydata if resize: inv_tr = self._get_rotation_transform().inverted() xdata, ydata = inv_tr.transform([xdata, ydata]) @@ -3707,6 +3740,7 @@ def _press(self, event): self.verts = [self._get_data(event)] self._selection_artist.set_visible(True) + @_call_with_reparented_event def _release(self, event): if self.verts is not None: self.verts.append(self._get_data(event)) @@ -3877,6 +3911,7 @@ def _update_box(self): # Save a copy self._old_box_extents = self._box.extents + @_call_with_reparented_event def _scale_polygon(self, event): """ Scale the polygon selector points when the bounding box is moved or @@ -3941,6 +3976,7 @@ def _press(self, event): # support the 'move_all' state modifier). self._xys_at_press = self._xys.copy() + @_call_with_reparented_event def _release(self, event): """Button release event handler.""" # Release active tool handle. @@ -3960,11 +3996,12 @@ def _release(self, event): elif (not self._selection_completed and 'move_all' not in self._state and 'move_vertex' not in self._state): - self._xys.insert(-1, self._get_data_coords(event)) + self._xys.insert(-1, (event.xdata, event.ydata)) if self._selection_completed: self.onselect(self.verts) + @_call_with_reparented_event def onmove(self, event): """Cursor move event handler and validator.""" # Method overrides _SelectorWidget.onmove because the polygon selector @@ -3988,17 +4025,16 @@ def _onmove(self, event): # Move the active vertex (ToolHandle). if self._active_handle_idx >= 0: idx = self._active_handle_idx - self._xys[idx] = self._get_data_coords(event) + self._xys[idx] = (event.xdata, event.ydata) # Also update the end of the polygon line if the first vertex is # the active handle and the polygon is completed. if idx == 0 and self._selection_completed: - self._xys[-1] = self._get_data_coords(event) + self._xys[-1] = (event.xdata, event.ydata) # Move all vertices. elif 'move_all' in self._state and self._eventpress: - xdata, ydata = self._get_data_coords(event) - dx = xdata - self._eventpress.xdata - dy = ydata - self._eventpress.ydata + dx = event.xdata - self._eventpress.xdata + dy = event.ydata - self._eventpress.ydata for k in range(len(self._xys)): x_at_press, y_at_press = self._xys_at_press[k] self._xys[k] = x_at_press + dx, y_at_press + dy @@ -4018,7 +4054,7 @@ def _onmove(self, event): if len(self._xys) > 3 and v0_dist < self.grab_range: self._xys[-1] = self._xys[0] else: - self._xys[-1] = self._get_data_coords(event) + self._xys[-1] = (event.xdata, event.ydata) self._draw_polygon() @@ -4040,12 +4076,12 @@ def _on_key_release(self, event): and (event.key == self._state_modifier_keys.get('move_vertex') or event.key == self._state_modifier_keys.get('move_all'))): - self._xys.append(self._get_data_coords(event)) + self._xys.append((event.xdata, event.ydata)) self._draw_polygon() # Reset the polygon if the released key is the 'clear' key. elif event.key == self._state_modifier_keys.get('clear'): event = self._clean_event(event) - self._xys = [self._get_data_coords(event)] + self._xys = [(event.xdata, event.ydata)] self._selection_completed = False self._remove_box() self.set_visible(True) @@ -4147,24 +4183,26 @@ def __init__(self, ax, xy, callback, *, useblit=True, props=None): self.connect_event('button_release_event', self.onrelease) self.connect_event('motion_notify_event', self.onmove) + @_call_with_reparented_event def onrelease(self, event): if self.ignore(event): return if self.verts is not None: - self.verts.append(self._get_data_coords(event)) + self.verts.append((event.xdata, event.ydata)) if len(self.verts) > 2: self.callback(self.verts) self.line.remove() self.verts = None self.disconnect_events() + @_call_with_reparented_event def onmove(self, event): if (self.ignore(event) or self.verts is None or event.button != 1 or not self.ax.contains(event)[0]): return - self.verts.append(self._get_data_coords(event)) + self.verts.append((event.xdata, event.ydata)) self.line.set_data(list(zip(*self.verts))) if self.useblit: