Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
maximlt opened this issue Apr 5, 2023 · 5 comments
Open

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

maximlt opened this issue Apr 5, 2023 · 5 comments
Labels
type-feature Feature request
Milestone

Comments

@maximlt
Copy link
Member

maximlt commented Apr 5, 2023

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 (#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 maximlt added the type-feature Feature request label Apr 5, 2023
@maximlt maximlt added this to the 2.0 milestone Apr 5, 2023
@MarcSkovMadsen
Copy link
Collaborator

My vote goes to 2.

I believe this keeps things simple.

@hoxbro
Copy link
Member

hoxbro commented Apr 6, 2023

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

@philippjfr
Copy link
Member

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.

@jlstevens
Copy link
Contributor

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).

@maximlt
Copy link
Member Author

maximlt commented Apr 12, 2023

I have opened #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 Deprecate passing positional arguments to Parameters #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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature Feature request
Projects
None yet
Development

No branches or pull requests

5 participants