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

Automatically add Optional type to kwargs with None default #30

Closed
mpariente opened this issue Dec 12, 2020 · 7 comments
Closed

Automatically add Optional type to kwargs with None default #30

mpariente opened this issue Dec 12, 2020 · 7 comments
Labels
enhancement New feature or request

Comments

@mpariente
Copy link

I was having a look a this PR and decided to give it a try.

# In parsing_workout.py
class Dummy:
    def __init__(self, dummy: float = None):
        pass

Then execute

python parsing_workout.py --print_config > conf.yml
python parsing_workout.py --config conf.yml

I get the following error :

parsing_workout.py: error: Parser key "data.dummy": float() argument must be a string or a number, not 'NoneType'

Notably, this doesn't crash with IPython.

I think it would make sense to automatically add Optional to the type in those cases, WDYT?

@mauvilsa
Copy link
Member

Automatically adding Optional would be an easy change. I agree it is nice that a None default is implicitly an optional, since adding the optional to the type seems kind of redundant. Though it seems that this implicit optional is discouraged and in the near future by default will not be a valid type hint, see python/mypy#9091 and python/peps#689. So I have a bit of mixed feelings about this.

@mpariente
Copy link
Author

Wasn't aware about this. I understand.
Maybe support an --implicit-optional flag as mypy is going to make?

@mauvilsa
Copy link
Member

I think that maybe we just add the implicit optional in jsonargparse without any option. Anyway mypy will show people the error when this is no longer a valid type. The ithe implicit optional code in jsonargparse would not be used, but at least it doesn't hurt.

@mauvilsa mauvilsa added the enhancement New feature or request label Dec 15, 2020
@mpariente
Copy link
Author

Sounds good.

@mauvilsa
Copy link
Member

This is now implemented. Please check.

@mauvilsa
Copy link
Member

This is now in release v3.2.0.

@mpariente
Copy link
Author

Thanks! I'll have a look soon

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

No branches or pull requests

2 participants