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

[Feature Request] Instantiate objects that require positional-only arguments #808

Closed
NTL-Remi opened this issue Jul 22, 2020 · 7 comments · Fixed by #1470
Closed

[Feature Request] Instantiate objects that require positional-only arguments #808

NTL-Remi opened this issue Jul 22, 2020 · 7 comments · Fixed by #1470
Labels
enhancement Enhanvement request wishlist Low priority feature requests

Comments

@NTL-Remi
Copy link

NTL-Remi commented Jul 22, 2020

🚀 Feature Request

Hello,
I propose to add the ability to specify positional-only arguments for object instantiation.

Motivation

Is your feature request related to a problem? Please describe.
Since Python 3.8, and more precisely PEP 570, it is possible to define functions that forbid passing arguments by name.
From what I have seen from the docs and the code (call(), _instantiate_class() and _get_kwargs()) it is only possible to assign a dict-like structure to the params key in the YAML file, that will end up being passed as **kwargs, so there is currently no way to give positional-only arguments through config files.

Pitch

Describe the solution you'd like
Replacing the params key by args and kwargs, taking respectively a list and a dict.
This will be breaking existing code, so maybe keep params as an alias to kwargs.
I leave open the question of whether the pass-through args should append, prepend or override the config args.

Describe alternatives you've considered
On the user side, wrapping each class or function that needs it in an adapter class/function that takes the same arguments but without the positional-only restriction.
I can see that getting tedious, and it kind of defeats PEP 570 's purpose.

Are you willing to open a pull request? (See CONTRIBUTING) <= btw, this link from the template is broken
Maybe, if I have time.

@NTL-Remi NTL-Remi added the enhancement Enhanvement request label Jul 22, 2020
@shagunsodhani
Copy link
Contributor

Thanks for the feature request. One problem I see is that we support Python 3.6 and 3.7 as well while position arguments are only supported since Python 3.8

@remisphere
Copy link

Well 3.6 and 3.7 still accept positional arguments, they are just not mandatorily positional.
So regarding the python version, there is no backward compatibility problem.

@remisphere
Copy link

Sorry, I am the same person, just answering from another account ><

@shagunsodhani
Copy link
Contributor

Oops :) I think I misunderstood the motivation. I assumed that you were requesting support for "positional-only" parameters.

@remisphere
Copy link

remisphere commented Jul 22, 2020

No, but from what I understand of the code, it is currently not possible to instantiate objects that have an __init__ with positional-only arguments.
From the config file at least.

@NTL-Remi NTL-Remi changed the title [Feature Request] Instantiate objects with positional-only arguments [Feature Request] Instantiate objects that require positional-only arguments Jul 22, 2020
@omry
Copy link
Collaborator

omry commented Jul 22, 2020

Thanks for the feature request.
The surface area of the instantiate method is already quite large and I am not sure this is worth the added complexity. Especially given that we plan to make the surface area even larger by supporting recursive instantiation (#566).

One workaround that you can employ now which is to use a static or class method as a factory, that will delegate the parameters appropriately for the underlying method.

I am going to keep it as a wishlist item, but this is not something that is likely to be added in the near future.
If this item gets many upvotes I may reconsider.

@omry omry added the wishlist Low priority feature requests label Jul 22, 2020
@omry
Copy link
Collaborator

omry commented Jul 24, 2020

Let's put a timeout of 12 months on this feature request. if there is no significant interest in 12 months I will close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhanvement request wishlist Low priority feature requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants