unwatch no longer logs a warning and idempotent behavior clarified (#1018)

Author: maximlt

doc/user_guide/Dependencies_and_Watchers.ipynb

@@ -480,7 +480,7 @@
     "\n",
     "- `obj.param.watch_values(fn, parameter_names, what='value', onlychanged=True, queued=False, precedence=0)`: <br>Easier-to-use version of `obj.param.watch` specific to watching for changes in parameter values. Same as `watch`, but hard-codes `what='value'` and invokes the callback `fn` using keyword arguments _param_name_=_new_value_ rather than with a positional-argument list of `Event` objects.\n",
     "\n",
-    "- `obj.param.unwatch(watcher)`: <br>Remove the given `Watcher` (typically obtained as the return value from `watch` or `watch_values`) from those registered on this `obj`.\n",
+    "- `obj.param.unwatch(watcher)`: <br>Remove the given `Watcher` (typically obtained as the return value from `watch` or `watch_values`) from those registered on this `obj`. Calling this method again after the watcher has already been unregistered is a no-op and won't raise any error.\n",
     "\n",
     "To see how to use `watch` and `watch_values`, let's make a class with parameters `a` and `b` and various watchers with corresponding callback methods:"
    ]

param/parameterized.py

@@ -3905,7 +3905,11 @@ def _spec_to_obj(self_, spec, dynamic=True, intermediate=True):
             deps.append(info)
         return deps, dynamic_deps
 
-    def _register_watcher(self_, action, watcher, what='value'):
+    def _register_watcher(
+        self_,
+        action: Literal['append', 'remove'],
+        watcher: Watcher, what: str = 'value',
+    ):
         if self_.self is not None and not self_.self._param__private.initialized:
             raise RuntimeError(
                 '(Un)registering a watcher on a partially initialized Parameterized instance '
@@ -3925,12 +3929,19 @@ def _register_watcher(self_, action, watcher, what='value'):
                     watchers[parameter_name] = {}
                 if what not in watchers[parameter_name]:
                     watchers[parameter_name][what] = []
-                getattr(watchers[parameter_name][what], action)(watcher)
+                method = getattr(watchers[parameter_name][what], action)
             else:
                 watchers = self_[parameter_name].watchers
                 if what not in watchers:
                     watchers[what] = []
-                getattr(watchers[what], action)(watcher)
+                method = getattr(watchers[what], action)
+            try:
+                method(watcher)
+            except ValueError:
+                # ValueError raised when attempting to remove an already
+                # removed watcher. Error swallowed as unwatch is idempotent.
+                if action != 'remove':
+                    raise
 
     def watch(
         self_,
@@ -4040,7 +4051,8 @@ def unwatch(self_, watcher: Watcher) -> None:
 
         This method unregisters a previously registered `Watcher` object,
         effectively stopping it from being triggered by events on the associated
-        parameters.
+        parameters. Calling unwatch with an already unregistered watcher
+        is a no-op.
 
         Parameters
         ----------
@@ -4080,11 +4092,12 @@ def unwatch(self_, watcher: Watcher) -> None:
         No callback is triggered after removing the watcher:
 
         >>> instance.a = 20  # No output
+
+        Calling unwatch() again has no effect:
+
+        >>> instance.param.unwatch(watcher)
         """
-        try:
-            self_._register_watcher('remove', watcher, what=watcher.what)
-        except Exception:
-            self_.warning(f'No such watcher {str(watcher)} to remove.')
+        self_._register_watcher('remove', watcher, what=watcher.what)
 
     def watch_values(
         self_,

tests/testwatch.py

@@ -8,7 +8,7 @@
 
 from param.parameterized import Skip, discard_events
 
-from .utils import MockLoggingHandler, warnings_as_excepts
+from .utils import MockLoggingHandler
 
 
 class Accumulator:
@@ -250,15 +250,8 @@ def accumulator(change):
         obj = SimpleWatchExample()
         watcher = obj.param.watch(accumulator, 'a')
         obj.param.unwatch(watcher)
-        with warnings_as_excepts(match='No such watcher'):
-            obj.param.unwatch(watcher)
-        try:
-            param.parameterized.warnings_as_exceptions = False
-            obj.param.unwatch(watcher)
-            self.log_handler.assertEndsWith('WARNING',
-                                ' to remove.')
-        finally:
-            param.parameterized.warnings_as_exceptions = True
+        # Idempotent, not error raised.
+        obj.param.unwatch(watcher)
 
     def test_simple_batched_watch_setattr(self):
 
@@ -720,6 +713,14 @@ def __init__(self, **params):
         P()
 
 
+    def test_watch_raises_bad_parameter(self):
+        obj = SimpleWatchExample()
+        with pytest.raises(
+            ValueError,
+            match="does_not_exist parameter was not found in list of parameters of class SimpleWatchExample"
+        ):
+            obj.param.watch(lambda e: print(e), 'does_not_exist')
+
 
 class TestWatchMethod(unittest.TestCase):