diff --git a/doc/api/next_api_changes/deprecations/30844-IHI.rst b/doc/api/next_api_changes/deprecations/30844-IHI.rst new file mode 100644 index 000000000000..55ebe9af6d68 --- /dev/null +++ b/doc/api/next_api_changes/deprecations/30844-IHI.rst @@ -0,0 +1,6 @@ +``CallbackRegistry.disconnect`` *cid* parameter renamed to *cid_or_func* +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +The *cid* parameter of `.CallbackRegistry.disconnect` has been renamed to +*cid_or_func*. The method now also accepts a callable, which will disconnect +that callback from all signals or from a specific signal if the *signal* +keyword argument is provided. diff --git a/doc/release/next_whats_new/callback_registry_disconnect_func.rst b/doc/release/next_whats_new/callback_registry_disconnect_func.rst new file mode 100644 index 000000000000..05825fe7e774 --- /dev/null +++ b/doc/release/next_whats_new/callback_registry_disconnect_func.rst @@ -0,0 +1,19 @@ +``CallbackRegistry.disconnect`` allows directly callbacks by function +------------------------------------------------------------------------- + +`.CallbackRegistry` now allows directly passing a function and optionally signal to +`~.CallbackRegistry.disconnect` instead of needing to track the callback ID +returned by `~.CallbackRegistry.connect`. + +.. code-block:: python + + from matplotlib.cbook import CallbackRegistry + + def my_callback(event): + print(event) + + callbacks = CallbackRegistry() + callbacks.connect('my_signal', my_callback) + + # Disconnect by function reference instead of callback ID + callbacks.disconnect('my_signal', my_callback) diff --git a/lib/matplotlib/cbook.py b/lib/matplotlib/cbook.py index a2a9e54792d9..763389ac61e5 100644 --- a/lib/matplotlib/cbook.py +++ b/lib/matplotlib/cbook.py @@ -229,6 +229,9 @@ class CallbackRegistry: >>> callbacks.process('drink', 123) drink 123 + >>> callbacks.disconnect(ondrink, signal='drink') # disconnect by func + >>> callbacks.process('drink', 123) # nothing will be called + In practice, one should always disconnect all callbacks when they are no longer needed to avoid dangling references (and thus memory leaks). However, real code in Matplotlib rarely does so, and due to its design, @@ -331,23 +334,45 @@ def _remove_proxy(self, signal, proxy, *, _is_finalizing=sys.is_finalizing): if len(self.callbacks[signal]) == 0: # Clean up empty dicts del self.callbacks[signal] - def disconnect(self, cid): + @_api.rename_parameter("3.11", "cid", "cid_or_func") + def disconnect(self, cid_or_func, *, signal=None): """ - Disconnect the callback registered with callback id *cid*. + Disconnect a callback. + Parameters + ---------- + cid_or_func : int or callable + If an int, disconnect the callback with that connection id. + If a callable, disconnect that function from signals. + signal : optional + Only used when *cid_or_func* is a callable. If given, disconnect + the function only from that specific signal. If not given, + disconnect from all signals the function is connected to. + + Notes + ----- No error is raised if such a callback does not exist. """ - self._pickled_cids.discard(cid) - for signal, proxy in self._func_cid_map: - if self._func_cid_map[signal, proxy] == cid: - break - else: # Not found - return - assert self.callbacks[signal][cid] == proxy - del self.callbacks[signal][cid] - self._func_cid_map.pop((signal, proxy)) - if len(self.callbacks[signal]) == 0: # Clean up empty dicts - del self.callbacks[signal] + if isinstance(cid_or_func, int): + if signal is not None: + raise ValueError( + "signal cannot be specified when disconnecting by cid") + for sig, proxy in self._func_cid_map: + if self._func_cid_map[sig, proxy] == cid_or_func: + break + else: # Not found + return + self._remove_proxy(sig, proxy) + elif signal is not None: + # Disconnect from a specific signal + proxy = _weak_or_strong_ref(cid_or_func, None) + self._remove_proxy(signal, proxy) + else: + # Disconnect from all signals + proxy = _weak_or_strong_ref(cid_or_func, None) + for sig, prx in list(self._func_cid_map): + if prx == proxy: + self._remove_proxy(sig, proxy) def process(self, s, *args, **kwargs): """ diff --git a/lib/matplotlib/cbook.pyi b/lib/matplotlib/cbook.pyi index f7959a6fd0bb..abd5196454c6 100644 --- a/lib/matplotlib/cbook.pyi +++ b/lib/matplotlib/cbook.pyi @@ -33,7 +33,10 @@ class CallbackRegistry: signals: Iterable[Any] | None = ..., ) -> None: ... def connect(self, signal: Any, func: Callable) -> int: ... - def disconnect(self, cid: int) -> None: ... + @overload + def disconnect(self, cid_or_func: int) -> None: ... + @overload + def disconnect(self, cid_or_func: Callable, *, signal: Any | None = ...) -> None: ... def process(self, s: Any, *args, **kwargs) -> None: ... def blocked( self, *, signal: Any | None = ... diff --git a/lib/matplotlib/tests/test_cbook.py b/lib/matplotlib/tests/test_cbook.py index 9b97d8e7e231..bfcab398d34e 100644 --- a/lib/matplotlib/tests/test_cbook.py +++ b/lib/matplotlib/tests/test_cbook.py @@ -292,6 +292,127 @@ def test_callback_wrong_disconnect(self, pickle, cls): # check we still have callbacks registered self.is_not_empty() + @pytest.mark.parametrize('pickle', [True, False]) + @pytest.mark.parametrize('cls', [Hashable, Unhashable]) + def test_callback_disconnect_func(self, pickle, cls): + # ensure we start with an empty registry + self.is_empty() + + # create a class for testing + mini_me = cls() + + # test that we can add a callback + self.connect(self.signal, mini_me.dummy, pickle) + self.is_not_empty() + + # disconnect by function reference + self.callbacks.disconnect(mini_me.dummy, signal=self.signal) + + # check we now have no callbacks registered + self.is_empty() + + @pytest.mark.parametrize('pickle', [True, False]) + @pytest.mark.parametrize('cls', [Hashable, Unhashable]) + def test_callback_disconnect_func_wrong(self, pickle, cls): + # ensure we start with an empty registry + self.is_empty() + + # create a class for testing + mini_me = cls() + + # test that we can add a callback + self.connect(self.signal, mini_me.dummy, pickle) + self.is_not_empty() + + # try to disconnect with wrong signal - should do nothing + self.callbacks.disconnect(mini_me.dummy, signal='wrong_signal') + + # check we still have callbacks registered + self.is_not_empty() + + # try to disconnect with wrong function - should do nothing + mini_me2 = cls() + self.callbacks.disconnect(mini_me2.dummy, signal=self.signal) + + # check we still have callbacks registered + self.is_not_empty() + + def test_callback_disconnect_func_redefined(self): + # Test that redefining a function name doesn't affect disconnect. + # When you redefine a function, it creates a new function object, + # so disconnect should not disconnect the original. + self.is_empty() + + def func(): + pass + + self.callbacks.connect(self.signal, func) + self.is_not_empty() + + # Redefine func - this creates a new function object + def func(): + pass + + # Try to disconnect with the redefined function + self.callbacks.disconnect(func, signal=self.signal) + + # Original callback should still be registered + self.is_not_empty() + + @pytest.mark.parametrize('pickle', [True, False]) + @pytest.mark.parametrize('cls', [Hashable, Unhashable]) + def test_callback_disconnect_func_all_signals(self, pickle, cls): + # Test disconnecting a callback from all signals at once + self.is_empty() + + mini_me = cls() + + # Connect to multiple signals + self.callbacks.connect('signal1', mini_me.dummy) + self.callbacks.connect('signal2', mini_me.dummy) + assert len(list(self.callbacks._func_cid_map)) == 2 + + # Disconnect from all signals at once (no signal specified) + self.callbacks.disconnect(mini_me.dummy) + + # All callbacks should be removed + self.is_empty() + + def test_disconnect_cid_with_signal_raises(self): + # Passing signal with a cid should raise an error + self.is_empty() + cid = self.callbacks.connect(self.signal, lambda: None) + with pytest.raises(ValueError, match="signal cannot be specified"): + self.callbacks.disconnect(cid, signal=self.signal) + + @pytest.mark.parametrize('pickle', [True, False]) + @pytest.mark.parametrize('cls', [Hashable, Unhashable]) + def test_callback_disconnect_func_selective(self, pickle, cls): + # Test selectively disconnecting a callback from one signal + # while keeping it connected to another + self.is_empty() + + mini_me = cls() + + # Connect same function to multiple signals + self.callbacks.connect('signal1', mini_me.dummy) + self.callbacks.connect('signal2', mini_me.dummy) + assert len(list(self.callbacks._func_cid_map)) == 2 + + # Disconnect from only signal1 + self.callbacks.disconnect(mini_me.dummy, signal='signal1') + + # Should still have one callback registered (on signal2) + assert len(list(self.callbacks._func_cid_map)) == 1 + assert 'signal2' in self.callbacks.callbacks + assert 'signal1' not in self.callbacks.callbacks + + # Disconnect from signal2 + self.callbacks.disconnect(mini_me.dummy, signal='signal2') + + # Now all should be removed + self.is_empty() + @pytest.mark.parametrize('pickle', [True, False]) @pytest.mark.parametrize('cls', [Hashable, Unhashable]) def test_registration_on_non_empty_registry(self, pickle, cls):