Adds support for required/ mandatory parameter values

[MarcSkovMadsen]

Fixes #1

The first commit is a Proof of concept to spark the design discussion. I've added a required argument as suggested by @philippjfr. I can see that @jbednar also has an interested in this issue.

[MarcSkovMadsen]

As I don't know the inner workings of the param code base and there is no developer guide, I don't know what is required from the code or tests.

There is no support for pre-commit either. I'm choked :-)

I would need some strong guidance to proceed. Thanks.

[MarcSkovMadsen]

There is a Long discussion about required for typing here https://mail.python.org/archives/list/[email protected]/thread/PYHMQEL7KKV7CJOPLY5BR6PTR4O5K7VU/

[jbednar]

Thanks. Let's hold off on this until #605 is merged, which should make this go more smoothly.

[maximlt]

Considering the API only of this feature, there are two options available with a) passing a sentinel value to default (Parameter(default=REQUIRED)) and b) adding a new required slot (Parameter(required=True)), b) being the approach chosen in this PR.

Overall I think I prefer b), the API looks cleaner to me eyes, it's easy to document, and I believe the implementation should be more straightforward (not having to fiddle with another sentinel value now #605 is merged and has added Undefined), the implementation suggested in this PR seems almost ready.
On the other hand, adding a new global slot does have some performance implications, using more memory.

@jlstevens what's your opinion?

[maximlt]

I have updated this PR, most importantly to make sure all the class Parameters are evaluated. I have also added some more tests and updated the existing ones.

The behavior allowed by required is very similar to what Python allows with * added to a function signature before a list of required keyword arguments:

The error raised by Param is similar to the one emitted by Python. The only difference being that it includes the class name (P.__init__(), note the P).

[jlstevens]

I'll review this PR in more detail tomorrow...

That said, my initial reaction is that while I don't object to required being an optional flag to parameters, I don't think I would ever use it myself: positional arguments seem to me to be the idiomatic way to ensure arguments are provided. Generally, I would lean towards enforcing required parameters that way at the Python level instead of relying on param for this sort of thing...

[maximlt]

I see your point. I think the point made by Marc on Discourse is that he wanted to avoid having to write boiler plate code.

What this PR doesn't address is what it means to pass the default value, e.g.

import param

class P(param.Parameterized):
    x = param.Parameter(required=True)  # default is None

p = P(x=None)  # should this raise an error?

Maybe you have an opinion on this @MarcSkovMadsen ?

[maximlt]

@jlstevens a potential advantage of having required as a new slot is that it could be leveraged by GUIs to label a widget as "mandatory" (with that red asterisk, you know)? Although I'm not sure the semantics match, as in a form you usually validate that the mandatory fields have been filled at some submit time, while in Param's case it has an effect earlier at the instantiation time. Now I'm not actually sure the two concepts can be combined in a single option! Might be worth considering that when deciding the fate of this PR.

[philippjfr]

Good point, that is definitely a very common use case for this functionality. It really requires a secondary piece of information, since it implies some action you need to know what the parameter is required for. The most common case is a form submission. For that case I could see an Event or an Action parameter having a list of required parameters associated with it.

[MarcSkovMadsen]

I see your point. I think the point made by Marc on Discourse is that he wanted to avoid having to write boiler plate code.

What this PR doesn't address is what it means to pass the default value, e.g.

import param



class P(param.Parameterized):

    x = param.Parameter(required=True)  # default is None



p = P(x=None)  # should this raise an error?

Maybe you have an opinion on this @MarcSkovMadsen ?

I can live with having to write a custom init function for now. I believe it would improve Param if this was documented as the recommended approach (for now).

I think that long term/ theoretically Params Parameters should have an api similar to Typing/ Dataclasses/ Pydantic such that eventually static type checking can be supported. If that enables supporting required parameters in forms it would be perfect.

[MarcSkovMadsen]

Just for reference there is a request here for Panel to support an api for forms here holoviz/panel#3687.

[maximlt]

I can live with having to write a custom init function for now. I believe it would improve Param if this was documented as the recommended approach (for now).

I'm not sure I understand. Are you saying that you actually don't need the feature you started to implement in this PR?

I think that long term/ theoretically Params Parameters should have an api similar to Typing/ Dataclasses/ Pydantic such that eventually static type checking can be supported. If that enables supporting required parameters in forms it would be perfect.

Indeed, but that is out of scope of this PR and discussion I believe.

To rephrase what I was asking you, do you believe that in the example below instantiating P with x set to None, which is the default value of that Parameter, should raise an error? I'm asking you in particular as in your example on Discourse you're mentioning that you have a class for which the default Parameters don't make sense.

import param

class P(param.Parameterized):
    x = param.Parameter(required=True)  # default is None

p = P(x=None)  # should this raise an error?

[MarcSkovMadsen]

The example should not raise an error because of required=True as you provide a parameter.

But if allow_None=False it should of course raise an exception.

[maximlt]

The example should not raise an error because of required=True as you provide a parameter.

But if allow_None=False it should of course raise an exception.

Ok thanks for your feedback, this is the behavior implemented in this PR.

[jbednar]

Thanks, @marc for the implementation and @maxime for the adjustments to it. I'm happy for this PR to be merged as-is; looks good to me!

For completeness, though, I think there is a third way to enforce required parameters not considered above, beyond (1) specifying a param.Required sentinel value for the default or (2) supporting required=True as in this PR. Specifically, now that Undefined lets us determine whether an argument has been passed in or not, we could consider (3) using allow_None to cover this case. Regardless of this PR or any thoughts about required parameters, I think an error should be raised if the user provides a default of None but explicitly specifies allow_None=False. We haven't previously raised such an error, because the default for allow_None is False, so we have silently overridden that declaration to match the new information that apparently None is allowed. Now that we can tell the user has explicitly set allow_None to False, I think we can reliably detect and signal this combination as an error, regardless of any considerations about required parameters. But then if we do raise an error there, we already have a poor-man's support for required Parameters, i.e. specifying the impossible combination default=None, allow_None=False that an instantiator must then override.

I'm not actually arguing for approach (3), because e.g. it wouldn't allow someone to accept None as a value for a required Parameter. But I did want to list it for the record.

[jlstevens]

In some ways I don't think something like this would be insane:

class Foo(param.Parameterized):
   foo = param.Number(default=param.RequiredValue(52.4))

The main reason I say this is that for all the years I have used param, I think this may be the first time where a new argument has been proposed to apply for all parameters.

While I am not suggesting that I want to block this PR given that some people may find it useful, I know that (personally) I will continue to enforce mandatory arguments in the class constructor explicitly. I think I find this a lot easier than having to scan through all the parameters for the required flag, especially as parameters can be inherited from super classes imported from different spots in the code.

[maximlt]

We had a chat with Philipp, this feels like it needs a bit more discussion and is a pretty minor feature that doesn't necessarily have to be bound to Param 2.0, so it's going to be deferred for a little while.