param icon indicating copy to clipboard operation
param copied to clipboard

No error raised when a method depends on Parameterized instances set after calling super()

Open maximlt opened this issue 2 years ago • 7 comments

See this example taken from Panel's user guide. The two widgets must be declared in the __init__ before calling super().

class P2(param.Parameterized):
    value = param.String()

class P1(param.Parameterized):
    
    def __init__(self, **params):
        # Works as expected when declared before
        # self.p2 = P2()
        super().__init__(**params)
        # Does not work when declared after super()
        # Raises an error in param<1.12, nothing for param 1.12
        self.p2 = P2()

    @param.depends('p2.value', watch=True)
    def cb(self):
        print('fired')

When they're declared after as in the example above, the following error used to be raised (this is param 1.11.1):

--------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-36-7e8d061f6e7f> in <module>
----> 1 p1 = P1()

<ipython-input-35-d76ac1c5846c> in __init__(self, **params)
      6     def __init__(self, **params):
      7 #         self.p2 = P2()
----> 8         super().__init__(**params)
      9         self.p2 = P2()
     10 

~/miniconda3/envs/holoviz37/lib/python3.7/site-packages/param/parameterized.py in __init__(self, **params)
   2610                 # 'dependers'.
   2611                 grouped = defaultdict(list)
-> 2612                 for dep in self.param.params_depended_on(n):
   2613                     grouped[(id(dep.inst),id(dep.cls),dep.what)].append(dep)
   2614                 for group in grouped.values():

~/miniconda3/envs/holoviz37/lib/python3.7/site-packages/param/parameterized.py in params_depended_on(self_, name)
   1858 
   1859     def params_depended_on(self_,name):
-> 1860         return _params_depended_on(MInfo(cls=self_.cls,inst=self_.self,name=name,method=getattr(self_.self_or_cls,name)))
   1861 
   1862 

~/miniconda3/envs/holoviz37/lib/python3.7/site-packages/param/parameterized.py in _params_depended_on(minfo)
    500     dinfo = getattr(minfo.method,"_dinfo", {})
    501     for d in dinfo.get('dependencies', list(minfo.cls.param)):
--> 502         things = (minfo.inst or minfo.cls).param._spec_to_obj(d)
    503         for thing in things:
    504             if isinstance(thing,PInfo):

~/miniconda3/envs/holoviz37/lib/python3.7/site-packages/param/parameterized.py in _spec_to_obj(self_, spec)
   1902         attr = m.group("attr")
   1903 
-> 1904         src = self_.self_or_cls if obj=='' else _getattrr(self_.self_or_cls,obj[1::])
   1905         cls,inst = (src, None) if isinstance(src, type) else (type(src), src)
   1906 

~/miniconda3/envs/holoviz37/lib/python3.7/site-packages/param/parameterized.py in _getattrr(obj, attr, *args)
    281     def _getattr(obj, attr):
    282         return getattr(obj, attr, *args)
--> 283     return reduce(_getattr, [obj] + attr.split('.'))
    284 
    285 

~/miniconda3/envs/holoviz37/lib/python3.7/site-packages/param/parameterized.py in _getattr(obj, attr)
    280 def _getattrr(obj, attr, *args):
    281     def _getattr(obj, attr):
--> 282         return getattr(obj, attr, *args)
    283     return reduce(_getattr, [obj] + attr.split('.'))
    284 

AttributeError: 'P1' object has no attribute 'p2'

Even though it's an error message that's quite hard to understand, at least there was an error. The current version on master doesn't raise any.

maximlt avatar Oct 29 '21 09:10 maximlt

Yes, it does seem good to have an error message in this case.

Note that in general we have add_parameter for adding parameters since param 1.12; I'm not sure what features will fail to work if the parameter is set directly on the instance like this (but presumably anything involving the class).

jbednar avatar Nov 09 '21 23:11 jbednar

@jbednar FYI I've simplified the example.

I wouldn't really know how to use add_parameter to add a parameter that's watched with param.depends. If you meant something like that:

class P2(param.Parameterized):
    value = param.String()

class P1(param.Parameterized):
    
    def __init__(self, **params):
        self.param._add_parameter('p2', P2().param.value)
        super().__init__(**params)
    
    @param.depends('p2.value', 'test', watch=True)
    def cb(self):
        print('fired')

it raises this error:

AttributeError: The String parameter 'value' has already been assigned a name by the P200142 class, could not assign new name 'p2'. Parameters may not be shared by multiple classes; ensure that you create a new parameter instance for each new class.

Maybe the cleanest way to declare that you want to depend on parameters of another parameterized object is as follows:

class P2(param.Parameterized):
    value = param.String()

class P1(param.Parameterized):
    p2 = param.Parameter()
    
    def __init__(self, **params):
        super().__init__(**params)
    
    @param.depends('p2.value', watch=True)
    def cb(self):
        print('fired')

p2 = P2()
p1 = P1(p2=p2)

maximlt avatar Nov 12 '21 10:11 maximlt

This doesn't raise any error with the latest version of param. It does if instead of depending on thing.whatever I declare that I depend on thing.

class P1(param.Parameterized):
    x = param.Number()
    
    @param.depends('thing.whatever', watch=True)
    def cb(self):
        print('fired')

maximlt avatar Nov 12 '21 10:11 maximlt

I wouldn't really know how to use add_parameter to add a parameter that's watched with param.depends

I'm not quite getting why that would be a good idea in any case. I would think this is the pattern for watching another class's parameter:

import param

class P2(param.Parameterized):
    value = param.String()
p2=P2()

class P1(param.Parameterized):
    p2 = param.Parameter(p2)
    
    @param.depends('p2.value', watch=True)
    def cb(self):
        print('fired')

p1 = P1()
p2.value="new"

fired

jbednar avatar Nov 12 '21 12:11 jbednar

This doesn't raise any error with the latest version of param. It does if instead of depending on thing.whatever I declare that I depend on thing.

I think that should clearly be an error; thing needs to exist, whether or not thing.whatever does.

jbednar avatar Nov 12 '21 14:11 jbednar

Agreed, that should be an error. The recent work on dynamic dependencies loosened the validation here but that is definitely a case where it should still error.

philippjfr avatar Nov 12 '21 16:11 philippjfr

Guess it's related to this issue:

class P(param.Parameterized):
    
    @param.depends('x.y', watch=True, on_init=True)
    def cb(self):
        print('cb')

Instantiating P with p = P() will fire the callback and print 'cb'. Note that the callback depends on a subobject (of nothing in practice). Depending on just 'x' without x being a Parameter rightfully raises an error.

maximlt avatar Nov 19 '21 17:11 maximlt

I'm guessing that some of the lack of errors above are covered by https://github.com/holoviz/param/issues/357 . In any case, for the original example, I do think we should be detecting and complaining about the lack of p2 by the time we get to super().

jbednar avatar May 12 '23 20:05 jbednar

I've now tightened the validation here to ensure it errors when a watcher is created based on an invalid dependency specification. This will occur both during instance creation and when something else tries to resolve invalid the method dependencies. Therefore I'll consider this done.

philippjfr avatar Aug 02 '23 15:08 philippjfr