Skip to content
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

Provide more control over the call order of watched methods #618

Open
sdc50 opened this issue Apr 13, 2022 · 9 comments · Fixed by #628
Open

Provide more control over the call order of watched methods #618

sdc50 opened this issue Apr 13, 2022 · 9 comments · Fixed by #628
Milestone

Comments

@sdc50
Copy link

sdc50 commented Apr 13, 2022

Between versions 1.11.1 and 1.12.1 the order in which methods that are decorated with param.depends are triggered was changed.

In [1]: import param

In [2]: class Test(param.Parameterized):
   ...:     s = param.String(default='default')
   ...: 
   ...:     @param.depends('s', watch=True)
   ...:     def on_s_update(self):
   ...:         print(self.s)
   ...: 
   ...: class Sub(Test):
   ...: 
   ...:     @param.depends('s', watch=True)
   ...:     def more_on_s_update(self):
   ...:         print(self.s.upper())
   ...: 
   ...: s = Sub()
   ...: s.s = 'test'
test
TEST

In [3]: param.__version__
Out[3]: '1.11.1'

vs.

image

Without a way to explicitly control the call order much of my code was in a delicate balance, and this has just wreaked havoc on it all.

There should be a way explicitly control the call order of depended on methods. In the past I had discussed with @philippjfr about adding a precedence argument to the depends decorator (and the watch method) so that callbacks can be sorted and called in a specified order when a parameter is triggered.

@philippjfr
Copy link
Member

Thanks for the issue, suspect this was a regression introduced in 1.12. We should restore the original behavior AND add the precedence argument.

@philippjfr
Copy link
Member

Will reopen since the precedence proposal hadn't been added yet.

@philippjfr
Copy link
Member

I'm somewhat hesitant to add precedence as a reserved kwarg to depends since it's not an unlikely argument name. Any other suggestions?

@jbednar
Copy link
Member

jbednar commented May 26, 2022

I guess priority has the same issue. But any compound word should be ok, e.g. call_prority or call_order or calling_order.

@maximlt
Copy link
Member

maximlt commented May 30, 2022

Oh. I looked that up but could not find it, is it documented somewhere that there are reserved kwargs? Does this just apply to when @param.depends is used to decorate a function? As in:

import param

class P(param.Parameterized):

    x = param.String()
    w = param.String()


p = P()

@param.depends(x=p.param.x, watch=p.param.w)
def foo(x, watch):
    print(x)

p.x = 'test'

@philippjfr
Copy link
Member

I don't know if we explicitly discuss reserved kwargs, certainly we do discuss watch and now we'd be proposing to add another argument. I don't think there are any others but I might be wrong.

@maximlt
Copy link
Member

maximlt commented May 30, 2022

Btw this is the error I get with the snippet above:

Traceback (most recent call last):
  File "mltmess/issue_arg_depends.py", line 15, in <module>
    p.x = 'test'
  File "/Users/mliquet/WORK/DEV/param/param/parameterized.py", line 367, in _f
    instance_param.__set__(obj, val)
  File "/Users/mliquet/WORK/DEV/param/param/parameterized.py", line 369, in _f
    return f(self, obj, val)
  File "/Users/mliquet/WORK/DEV/param/param/parameterized.py", line 1248, in __set__
    obj.param._call_watcher(watcher, event)
  File "/Users/mliquet/WORK/DEV/param/param/parameterized.py", line 2039, in _call_watcher
    self_._execute_watcher(watcher, (event,))
  File "/Users/mliquet/WORK/DEV/param/param/parameterized.py", line 2021, in _execute_watcher
    watcher.fn(*args, **kwargs)
  File "/Users/mliquet/WORK/DEV/param/param/parameterized.py", line 446, in cb
    return func(*args, **dep_kwargs)
TypeError: foo() missing 1 required positional argument: 'watch'

@philippjfr
Copy link
Member

Right we need to explicitly validate reserved kwargs.

@maximlt maximlt modified the milestones: v1.12.2, v1.12.3 Jun 2, 2022
@maximlt maximlt modified the milestones: v1.12.x, v2.x Apr 4, 2023
@sdc50
Copy link
Author

sdc50 commented Aug 22, 2023

related discussion in this issue: holoviz/panel#2556

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants