param icon indicating copy to clipboard operation
param copied to clipboard

`name` of a Parameterized instance overridden when is the default of a `readonly/constant` Parameter

Open maximlt opened this issue 1 year ago • 1 comments

import param

class A(param.Parameterized):
    x = param.Number()

a1 = A(name="FOO1")
a2 = A(name="FOO2")

class B(param.Parameterized):
    const = param.Parameter(a1, constant=True)
    ro = param.Parameter(a2, readonly=True)

b = B()

print(f'{b.const.name=}')
print(f'{b.ro.name=}')

Param 1.13:

b.const.name='A00005'
b.ro.name='FOO2'

Now:

b.const.name='A00005'
b.ro.name='A00006'

The name of a Parameterized instance set as the default of a readonly Parameter is unexpectedly overridden. Bisecting pointed at https://github.com/holoviz/param/pull/776 that stopped automatically setting instantiate to True when constant is True.

However, I'm also surprised to see the name being updated for a constant Parameter! As currently the default value of a Parameter is deep copied only when instantiate=True. So in the example above, instantiate is always False, there's no deep copy of a1ora2`.

https://github.com/holoviz/param/blob/dae8fc4a7b47079468ad5da0b486a779e963351b/param/parameterized.py#L1922-L1926

So the change I'd suggest would be to override the Parameter name only when a deep copy is required which doesn't break any test and would lead to this output for the example:

b.const.name='FOO1'
b.ro.name='FOO2'

The piece of code in Parameterized._instantiate_param that overrides the name of a Parameterized class on instantiation originates from the first commit on Github. One could argue that, even when deep-copying, overriding name isn't correct and could be left as a responsibility of the users implementing their Parameterized classes.


Did I already say I'd like name to be removed from Param? :)

maximlt avatar Apr 19 '24 16:04 maximlt

Did I already say I'd like name to be removed from Param?

That's a different issue. Maybe Parameterized can be renamed to ParamObject and its name handling code moved into a new subclass now named Parameterized. That way downstream libraries can choose whether or not they wish their objects to have names, (e.g. from param import ParamObject as Parameterized) making it easier to retain backwards compatibility while removing the special name handling in practice. Feel free to file a separate issue and/or PR!

Meanwhile, for this specific issue, I do agree the behavior is both surprising and wrong. The proposed fix seems better, but my brain starts to overheat if I try to think of whether it truly addresses the issue. I'd ask another Param developer like @philippjfr .

jbednar avatar Apr 26 '24 21:04 jbednar

Maybe Parameterized can be renamed to ParamObject and its name handling code moved into a new subclass now named Parameterized.

Agreed, although not 100% aligned on the naming.

philippjfr avatar May 15 '24 13:05 philippjfr