param icon indicating copy to clipboard operation
param copied to clipboard

Make all (but one?) of the Parameter arguments keywords only

Open maximlt opened this issue 2 years ago • 5 comments

Parameters can currently be constructed with a variable number of positional arguments.

param.List([1, 2], int, int, True, (2, 4))
param.ObjectSelector(1, [1, 2, 3])

PEP 3102 implemented in Python 3.0 added support for declaring arguments that are keywords-only:

def compare(a, b, *, key=None):
    pass

# key must be passed as a keyword argument
compare(1, 2, key=operator.eq)

Param dates from way before this PEP was accepted and does not benefit from it, while I think it should :)

There's however a remaining question, that is about the first argument. @jbednar explained me that the spirit of the current state is that the first argument is the value that makes the most sense for the Parameter at hand. For instance, for Number it is the default number, or for Boolean the default boolean value. In most cases, default is indeed the first argument and can be passed by position, for example param.Boolean(True). There are a few Parameters that don't have default as their first argument:

  • ClassSelector has class_
  • Selector has objects. Note that its subclasses - ListSelector, FileSelector, and MultiFileSelector - still have default as a first argument :/ Same for ObjectSelector that is deprecated but still used a lot.
  • Composite has attribs
  • Dynamic actually has default as its first argument but it accepts it as keyword only

Now there are multiple ways to approach that, all have their pros and cons, and all are more or less a breaking change:

  1. all arguments keyword only
  2. all arguments keyword only but the first one that is always default
  3. all arguments keyword only but the first one that is the one that makes sense (mostly default, but can also be objects, class_, ...)

This is a timely question with the upcoming Param 2.0 and the Sentinel work (https://github.com/holoviz/param/pull/605) that touched the signatures.

Will need the opinion of Param users on this one to get a feeling on what the best API could be and what the extent of the breaking change you would be willing to accept: @jlstevens, @philippjfr, @Hoxbro, @MarcSkovMadsen

maximlt avatar Apr 05 '23 17:04 maximlt

My vote goes to 2.

I believe this keeps things simple.

MarcSkovMadsen avatar Apr 05 '23 17:04 MarcSkovMadsen

My vote is also on number 2 for the same reason as @MarcSkovMadsen.

Edit: My vote is changed to number 1 for the same reason as @philippjfr

hoxbro avatar Apr 06 '23 18:04 hoxbro

I think the only way forward we have here is to move to keyword only. There's no reasonable route to standardizing on consistently making the default argument the first positional argument because we cannot easily distinguish the cases where class_ or objects is currently the first argument. So I'd say 1. is the only real option in the short term, and once we have done that we can eventually re-add default as the first argument.

philippjfr avatar Apr 12 '23 08:04 philippjfr

I would personally be happy with option 1 and that seems like the simplest way to me as well.

Option 2 is also very reasonable and may feel more natural for some users. That said, I agree with Philipp in that I only think this should be done if the semantics for a positional argument are completely clear for all parameters (which is not the case right now).

jlstevens avatar Apr 12 '23 08:04 jlstevens

I have opened https://github.com/holoviz/param/pull/737 for you to see how it feels to have a warning emitted when a positional argument is passed to a Parameter.

# foo.py
import param

class P(param.Parameterized):
    n = param.Number(1)

Output:

foo.py:4: DeprecationWarning: Passing 'default' as positional argument(s) to 'param.Number' was deprecated, please pass them as keyword arguments.
  n = param.Number(1)

If we pursue with this approach, the plan I'd suggest would be to:

  1. merge #737 before Param 2.0 is release; the documentation will also need to be updated
  2. make the arguments keywords-only in Param 3.0
  3. eventually make default the only allowed positional argument in Param 3.1

maximlt avatar Apr 12 '23 17:04 maximlt