holoviz / param

Param: Make your Python code clearer and more reliable by declaring Parameters
https://param.holoviz.org
BSD 3-Clause "New" or "Revised" License
412 stars 69 forks source link

Do not trigger event if identity of parameter value is unchanged #901

Open philippjfr opened 5 months ago

philippjfr commented 5 months ago

Seems like a crazy oversight, not all objects can be compared for equality so we should always check for identity first. The current behavior was added in https://github.com/holoviz/param/pull/279, but it does not provide any arguments for why we would want to trigger events if the object identity is unchanged.

jbednar commented 5 months ago

It certainly sounds safe to add this check.

maximlt commented 5 months ago

I thought we didn't compare for identity because of mutable/complex objects. Take the code below, right now the debug callback is called. With your changes, it no longer is.

import param

class Dummy: pass

dummy = Dummy()

class Foo(param.Parameterized):
    d = param.ClassSelector(class_=Dummy)

    @param.depends('d', watch=True)
    def debug(self):
        print('updated d')

foo = Foo(d=dummy)
dummy.changed = True
foo.d = dummy  # triggers `debug` now, not with this PR

We could decide that's the right course of action, if so, I don't think it should be released in a patch release.

philippjfr commented 5 months ago

Thanks, that's a good point. Personally I don't think this was intentional but I appreciate that it's a significant change in behavior. For mutable objects the correct way to trigger an event should always be to explicitly .param.trigger an event. That is because if we ever did register an equality comparison for a particular type of object then the event would no longer get triggered because the equality check will always pass if the object has the same identity.

philippjfr commented 5 months ago

As an example to illustrate this point, the two mutables that we do have equality comparisons defined for (list and dict) do not trigger an event:

import param

class Foo(param.Parameterized):
    d = param.List()

    @param.depends('d', watch=True)
    def debug(self):
        print('updated d')

mutable = []

foo = Foo(d=mutable)
mutable.append(1)
foo.d = mutable # does not trigger `debug`
philippjfr commented 5 months ago

The docs about trigger also make this point:

Usually, a Watcher will be invoked only when a parameter is set (and only if it is changed, by default). What if you want to trigger a Watcher in other cases? For instance, if a parameter value is a mutable container like a list and you add or change an item in that container, Param’s set method will never be invoked, because in Python the container itself is not changed when the contents are changed. In such cases, you can trigger a watcher explicitly, using .param.trigger(*param_names).

maximlt commented 5 months ago

Yes, your points make sense to me.

We often see Param users, and in particular Panel users using Param, asking why updating their list/dict doesn't execute their callbacks. Expanding this behavior to "complex" (there's got to be a better name for this) will need to be carefully documented (with a big warning!).