Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/user_guide/Dependencies_and_Watchers.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -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:"
]
Expand Down
29 changes: 21 additions & 8 deletions param/parameterized.py
Original file line number Diff line number Diff line change
Expand Up @@ -3548,7 +3548,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 '
Expand All @@ -3568,12 +3572,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_,
Expand Down Expand Up @@ -3683,7 +3694,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
----------
Expand Down Expand Up @@ -3723,11 +3735,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_,
Expand Down
21 changes: 11 additions & 10 deletions tests/testwatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from param.parameterized import Skip, discard_events

from .utils import MockLoggingHandler, warnings_as_excepts
from .utils import MockLoggingHandler


class Accumulator:
Expand Down Expand Up @@ -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):

Expand Down Expand Up @@ -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):

Expand Down
Loading