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

Redefined param in child Parameterized class inherits the 'bounds' of the parameter in the parent class #837

Closed samimia-swks closed 9 months ago

samimia-swks commented 9 months ago

ALL software version info

v2.0.0a3

Description of expected behavior and the observed behavior

Say I have a child Parametrized class which inherits from a parent Parameterized class and redefines one of its parameters from a param.Integer to a param.List. If the param.Integer had a bounds defined, the param.List in the child class seems to inherit those bounds, which doesn't really make sense (the bounds of an integer enforce a range on the number, but the bounds of a list enforce a range on the length of the list).

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

import param

class parent(param.Parameterized):
    p = param.Integer(None, bounds=(1, None))

class child(parent):
    p = param.List([])

p = parent()
c = child()

The above code fails with ValueError: List parameter 'child.p' length must be at least 1, not 0. I expect p = param.List([]) to completely override p = param.Integer(None, bounds=(1, None)), and a default value of [] to be valid.

jbednar commented 9 months ago

Thanks for raising this issue; we definitely need to document this better! Behavior in this case is intentionally undefined because this is not supported code. Subclasses should never have parameters with a fundamentally different type than a superclass parameter of the same name. I believe there is an open issue about detecting that usage and warning or raising an error, though I can't find it just now.

For background, properly structured Python subclasses should satisfy the "is a" relationship: Any code that accepts type parent should be happy to get type child, because here child has been declared to be a (sub)type of parent (it inherits the interface of parent). See the definition of is_instance in the Python docs for the semantics involved. Here, c is (also) an instance of type parent, even though it's been declared as child, which for this code is a problem because a child does not satisfy the interface of the parent class:

image

Code written for parent will expect to see an integer value for p, but here it will be a list, immediately causing bugs.

That doesn't mean that you can't change types across the inheritance hierarchy. E.g. you can have a Number parameter in the parent become an Integer parameter in the child, since any valid integer value is also a valid number. But you can't have the same parameter be an Integer in the parent and a Number in the child, because then if someone provides an instance of the child to code written for the parent (which is fully legal by Python typing rules, since a child here "is a" (specific instance of) parent), that code will find arbitrary numbers where only Integers were declared to be allowed. Param is designed to avoid such surprises, so don't write code that causes them!

Changing an Integer to a List is even less appropriate, because code written for an Integer will essentially never work if it finds a List. So either don't have child inherit from parent (e.g. have them both inherit the truly shared bits from a third class), or change the name of p in the child so that it is not purporting to be the same parameter p as in the parent, or otherwise rewrite the code to avoid this error.

samimia-swks commented 9 months ago

I see. Thanks for the detailed explanation.

maximlt commented 9 months ago

We could improve the error message here. The error happens while creating the child class when it detects that the type of a Parameter has changed. In that case we could catch the validation error (a mix of ValueError, TypeError OSError) and enhance it with some more context.