param icon indicating copy to clipboard operation
param copied to clipboard

param.depends(on_init=True) does not execute without watch=True

Open phi6ias opened this issue 2 years ago • 8 comments

Using the param.depends decorator in a param.Parameterized class with on_init=True does not execute without also setting watch=True.

ALL software version info

python 3.11.1 param 1.12.3

run in jupyter notebook within VS Code Version: 1.75.1 (user setup) Commit: 441438abd1ac652551dbe4d408dfcec8a499b8bf Date: 2023-02-08T21:32:34.589Z Electron: 19.1.9 Chromium: 102.0.5005.194 Node.js: 16.14.2 V8: 10.2.154.23-electron.0 OS: Windows_NT x64 10.0.19045 Sandboxed: No

Description of expected behavior and the observed behavior

expected behaviour would be that a function with decorator "@param.depends(on_init=True)" would execute on init without extra call or setting watch=True.

Complete, minimal, self-contained example code that reproduces the issue

import param

class Oninit(param.Parameterized):
    a = param.String(default="not initialised")

    def __init__(self, **params):
        super().__init__(**params)
        print("Oninit, __init__, 1: ", self.a)
        # Calling the fn directly works:
        # self.dosomethingoninit()
        print("Oninit, __init__, 2: ", self.a)
        return

    # That decorator leads to a function call:
    # @param.depends(watch=True, on_init=True)
    # That decorator doesn't execute:
    @param.depends(on_init=True)
    def dosomethingoninit(self):
        print("Oninit, dosomethingoninit, 1: ", self.a)
        self.a = "did something"
        print("Oninit, dosomethingoninit, 2: ", self.a)
        return

oi = Oninit()

Stack traceback and/or browser JavaScript console output

@param.depends(on_init=True) results in:

Oninit, __init__, 1:  not initialised
Oninit, __init__, 2:  not initialised

@param.depends(watch=True, on_init=True) results in:

Oninit, dosomethingoninit, 1:  not initialised
Oninit, dosomethingoninit, 2:  did something
Oninit, __init__, 1:  did something
Oninit, __init__, 2:  did something

calling function directly inside __init() results in:

Oninit, __init__, 1:  not initialised
Oninit, dosomethingoninit, 1:  not initialised
Oninit, dosomethingoninit, 2:  did something
Oninit, __init__, 2:  did something

Screenshots or screencasts of the bug in action

phi6ias avatar Feb 15 '23 20:02 phi6ias

It does seem to me like it should be initialized in this case. @philippjfr , I think you had looked at on_init use cases; do you also agree?

jbednar avatar Feb 15 '23 20:02 jbednar

Definitely agree, on_init should be independent from watching for changes.

philippjfr avatar Feb 15 '23 21:02 philippjfr

I won't have time to address this soon, but a quick search for on_init in the source code suggests that there is at least one case where its testing for watch and then returning; could be a straightforward PR to attempt.

jbednar avatar Feb 16 '23 01:02 jbednar

My take on this one is that I believe this requires to be a little more specified.

on_init was implemented after https://github.com/holoviz/param/issues/513 except that the OP required for the callback to be triggered only when the dependent Parameter was updated in the constructor, they re-iterated their request https://github.com/holoviz/param/issues/616 which was partially implemented in https://github.com/holoviz/param/pull/620 (Philipp told me he wasn't yet convinced by the utility of this change).

Reducing the example of the OP to its minimum, it seems to me they're asking for a way to do some action after the __init__ has executed, independently of any Parameter dependency. In that case, I'm not sure we should overload param.depends (which has a clear intent given its name) with the ability to just run some method on post-instantiation. Also, I can't think of any time where I have needed on_init=True without watch=True.

import param

class Oninit(param.Parameterized):
    a = param.String(default="not initialised")

    @param.depends(on_init=True)
    def dosomethingoninit(self):
        self.a = "did something"

oi = Oninit()

Quickly looking around, Python dataclasses allow their users to define a __post_init__ method that gets called after instantiation:

@dataclass
class C:
    a: float
    b: float
    c: float = field(init=False)

    def __post_init__(self):
        self.c = self.a + self.b

Maybe Param could get some inspiration from this approach to implement that feature request?

Additionally, I'll note that:

  • when @param.depends is used to decorate a function and not a method, on_init can be a tad confusing and seems to anyway have no effect in this context.
  • there's been some discussion about allowing the default Parameter attribute to accept a callback whose returned value would be used to set the actual default value. There's some potential overlap with this feature request.

Overall I'd suggest we move this to post Param 2.0. It'd be valuable to more clearly delineate how to:

  • set up default values
  • control how callbacks are executed

cc @jbednar

maximlt avatar Jul 19 '23 14:07 maximlt

It seems to me that #620 should be closed, since there has been no response to the questions and issues raised on it.

Regardless, what this issue is requesting seems like a straightforward fix to the code -- the code indicates that the method should be invoked on init, which is not happening. I don't think we have to solve any other issue than simply ensuring that the code is executed on init as requested, do we?

I.e., do we have to conflate this issue with a different OP's original goals?

For the __post_init__ example, in Param isn't that just:

    def __init__(self, **params):
        super().__init__(**params)
        self.c = self.a + self.b

Adding a post-init hook is a less explicit way to save a few characters; I don't object but don't think it adds much, and doesn't address the clearly incorrect behavior for on_init=True, watch=False.

when @param.depends is used to decorate a function and not a method, on_init can be a tad confusing and seems to anyway have no effect in this context.

I don't recommend @param.depends for decorating functions, so I don't have much opinion on what it does there. Functions don't have init, so whether it errors or ignores that argument seems arbitrary.

jbednar avatar Jul 21 '23 03:07 jbednar

Adding a post-init hook is a less explicit way to save a few characters; I don't object but don't think it adds much

You can't read my brain (yet) so I'll explain why I start to mention __post_init__. dataclasses implement that, I believe for the good reason that they automatically generate an __init__ method based on the declared fields, dataclasses also provide ways to define whether you want a field to be in the signature or not (init=False). Static type tools have ways to understand dataclasses, and other libraries that respect that special spec, so instantiating a dataclass in an IDE nicely offers you all the class fields. This is not the case for Parameterized classes, i.e. in an IDE you don't get very useful information, e.g. that's what I see in VSCode: (**params: Any).

This is far from being a complete plan though!


I guess yes that we should fix the reported issue. I would just hope that we don't document that. After all, when you decorate a method with @param.depends(on_init=True) you're declaring a dependency on all the Parameters of that class, I'm not sure this is the intention of the user.

I would not want to block the Param 2.0 release to get this implemented though, this seems like an edge case and it has a clear workaround. I'm moving this to 2.x, feel free to move it back to 2.0 if you deem this must be in the release.

maximlt avatar Jul 24 '23 19:07 maximlt

After all, when you decorate a method with @param.depends(on_init=True) you're declaring a dependency on all the Parameters of that class, I'm not sure this is the intention of the user.

Methods already depend on all Parameters of the class unless otherwise specified, so such a declaration doesn't change dependencies, only whether the method is invoked on init.

jbednar avatar Jul 25 '23 05:07 jbednar

Oh right! I never get this right! so indeed @param.depends(on_init=True) would not declare any dependency.

maximlt avatar Jul 25 '23 15:07 maximlt