factory function for Parameter instead of default value + instantiate=True

[tomas16]

Is your feature request related to a problem? Please describe.

I'm trying to use composition of 2 parameterized classes in which the top-level class has methods that depend on parameters in the subobjects. The subobjects have a view attribute which is essentially pn.Param(self.param) with some options for the widgets.

Below code doesn't work because when pm.depends is evaluated, self.a doesn't exist yet, so 'a.value' can't get evaluated (it generates an AttributeError). This is still true when self.a = A() happens before super().__init__().

import param as pm

class A(pm.Parameterized):
    value = pm.Parameter(default=123)

class B(pm.Parameterized):
    a = pm.Parameter()

    def __init__(self):
        super().__init__()
        self.a = A()

    @pm.depends('a.value', watch=True)
    def _value_update(self):
        print(self.a.value)

if __name__ == '__main__':
    b = B()
    b.a.value = 456

I tried instantiating an A object as a default value instead, redefining class B like this:

class B(pm.Parameterized):
    a = pm.Parameter(default=A(), instantiate=True)

    @pm.depends('a.value', watch=True)
    def _value_update(self):
        print(self.a.value)

This "works" in this toy example, but when A has a view attribute that contains panel GUI controls, the GUI controls don't work anymore. I suspect this is due to the way the default A instance is deepcopied when B objects are instantiated.

Describe the solution you'd like

Instead of having a default value that gets deepcopied, I propose supplying the Parameter constructor with a factory function to create a default value. It sidesteps all issues in cases where copy behavior gets complicated.

Describe alternatives you've considered

In my case, I was able to solve the problem by making A's view attribute a cached property. In python 3.7:

@property
@functools.lru_cache(maxsize=None)
def view(self):
    ...

[MarcSkovMadsen]

Hi @tomas16

If you provide A() to super().__init__ then it will work

import param as pm

class A(pm.Parameterized):
    value = pm.Parameter(default=123)

class B(pm.Parameterized):
    a = pm.Parameter()

    def __init__(self):
        super().__init__(a=A())

    @pm.depends('a.value', watch=True)
    def _value_update(self):
        print(self.a.value)

if __name__ == '__main__':
    b = B()
    b.a.value = 456

$ python 'script.py'
456

Feel free to reopen the issue if you think this does not solve your issue. Thanks.

[tomas16]

Thanks, that worked! Don't know how I missed that 🙂

[jbednar]

Thanks for the suggestion, @MarcSkovMadsen , and I'm glad it works here. Still, it sounds to me like there are underlying issues to address here. I think we should at least consider if we can make the dependency handling cover this case. I'd also at least like to see what the issue is with instantiate=True for panel, as that seems like a more straightforward implementation to me.

[tomas16]

If I can weigh in with the perspective of a relatively new holoviz user.

1. instantiate, while well-documented, is a misleading term since it means making a copy of the default value.
2. I guess the reason why I didn't try to provide my instance to super().__init__() is because the Parameter constructor seems to provide what I needed through default and instantiate and the documentation emphasizes that pattern.
3. Since there is a solution, the remaining issues seem more related to style (where should one instantiate this object) and documentation.
4. If you wanted to move the instantiation to the Parameter constructor, I still feel like making (deep) copies is potentially dangerous, and the factory function I suggested has examples in the standard library (e.g. collections.defaultdict takes a default_factory argument).
5. I understand how default + instantiate is useful for simple objects like lists or dicts containing simple types. This is the example in the documentation.
6. If at all possible, I would rename instantiate to something more appropriate, regardless of whether a factory function option gets added.

[MarcSkovMadsen]

Agree on instantiate. That name has always confused me.

[jbednar]

1. Yes, instantiate is a lousy name. A more accurate name would be deepcopy_default_value_per_parameterized_instance, but it's been called instantiate for many years now, so the best we could do is add an alias. Would people guess what it does if it were called deepcopy?
2. I agree; the Parameter constructor should already handle this case, though there may be some technical reason I can't see why that's infeasible. Definitely worth trying to see if we can address that!
3. Sure. We should at least document Marc's pattern if we take no other action.
4. Making deepcopies can indeed be dangerous, which is why it is not the default, but writing a factory function seems like a lot of work for something that is usually covered by a simple boolean flag. Seems ok to support such a factory function, but not for it to be the only way to control instantiation.
5. default + instantiate is useful not just for simple containers of literals, but also for deeply nested Parameterized objects. As long as the main state of such an object is in Parameters, instantiate=True ensures that instances do not share state. And instantiate=False covers when you do want a single shared state. I'd be interesting in seeing concrete examples not covered by one of those two options.

[tomas16]

Re 4:

• Agreed a factory shouldn't replace what's currently there. A user would provide either a default or a factory.
• The factory could simply be a class, so in my original example the parameter a in class B would be declared as a = pm.Parameter(default_factory=A)

[jbednar]

I'd entirely forgotten about it, but it turns out we've already got support for this. :-/ However, it's only implemented for Selector and subclasses:

https://github.com/holoviz/param/blob/d88921cc1992cf0e55d7289d57e90d96ce1c4e71/param/__init__.py#L1237

Assuming that this behavior is what you'd like (but for non-Selector types), I'd be happy to review and accept a PR that bubbles this functionality up higher, into Parameter, so that it is available for all Parameters.

[maximlt]

deepcopy_default_value_per_parameterized_instance is a little long and deepcopy a little too short :) (I'm always confused with readonly and constant that both lack some context). How about deepcopy_default or deepcopy_default_on_init? I believe that any of these suggestions is better than the current instantiate.

[MarcSkovMadsen]

For the record this is also requested and discussed in There is no default_factory for Param objects? on Discourse.

[jbednar]

Any volunteers to make a PR for moving the compute_default_fn functionality from Selector to Parameterized and renaming it to default_factory, and a separate PR adding an alias for instantiate with a better name (deepcopy_default?) and updating the docs to mention the new name rather than instantiate?