search

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

Ensure class watchers are not inherited by instance parameter #833

Closed philippjfr closed 9 months ago

philippjfr commented 9 months ago

In https://github.com/holoviz/param/pull/826 we discovered some weird logic that was copying over watchers from class to instance parameters. After discussing this we all agreed this does not make sense, this is because slots are not shared between class and instance parameters and therefore it does not make sense that an instance parameter would inherit the watchers from a completely different Parameter instance. Worse yet, once an instance was created the watcher would be removed from the class which means that modifying the class Parameter slot value would no longer trigger the watcher.

The correct behavior is actually quite simple, watchers should not be copied over, instead the Parameter slot watchers should be reset when the instance parameter is created. This ensure that while the instance still shares the Parameter object with the class it correctly fires if a slot on the class Parameter is changed and when an instance parameter is created only newly created watchers on the instance will be watched. Since calling inst.param.watch will automatically create instance parameters for any parameters that are watched this means that the behavior will be correct in all cases both instances and classes will always correctly trigger the appropriate watchers.

Fixes https://github.com/holoviz/param/issues/829

philippjfr commented 9 months ago

So it turns out the copying was effectively a hack to allow watchers that are set up during instance creation (i.e. primarily ones set up using @param.depends on a method) to be copied over after the instance is fully set up. I'm going to revert my latest changes which were forcing the creation of an instance parameter even before the class was fully initialized since it can potentially cause bad side-effects should someone try to set up a watcher before they call super(Parameterized self).__init__. Instead I will declare the instance initialized before we call _update_deps which is what is responsible for setting up the dependencies. This will ensure that watchers will be correctly installed on the instance parameter rather than the class parameter.

maximlt commented 9 months ago

I ran Panel and HoloViews unit test suites successfully against this branch. Merging, thanks!