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

Add options parameter that replaces strict_kwonly #92

Closed
wants to merge 17 commits into from

Conversation

mcarans
Copy link

@mcarans mcarans commented Nov 3, 2021

The strict_kwonly flag only affects parameters that have a default value - when set to False, parameters with default values are accepted as keyword arguments instead of postional arguments.

Deprecate strict_kwonly and replace by options="has_default" (i.e. strict_kwonly=False), ="kwonly" (strict_kwonly=True), "all" (all parameters accepted as keyword arguments)

lib/defopt.py Outdated Show resolved Hide resolved
lib/defopt.py Show resolved Hide resolved
lib/defopt.py Outdated Show resolved Hide resolved
lib/defopt.py Outdated Show resolved Hide resolved
lib/defopt.py Outdated Show resolved Hide resolved
test_defopt.py Outdated Show resolved Hide resolved
@anntzer
Copy link
Owner

anntzer commented Nov 4, 2021

As a side point, looks like a recent setuptools release broke CI. Let's see whether they fix this quickly, otherwise I'll have to fiddle with the CI config :/

@anntzer
Copy link
Owner

anntzer commented Nov 4, 2021

I bumped the dependency to docutils>=0.12 to fix the build, but you'll need to rebase the PR.

@mcarans
Copy link
Author

mcarans commented Nov 5, 2021

@anntzer Thanks for all the good suggestions. I've done them all. Please take a look.

lib/defopt.py Outdated Show resolved Hide resolved
lib/defopt.py Outdated Show resolved Hide resolved
lib/defopt.py Outdated Show resolved Hide resolved
…) is .... 'all' turns all parameters into command-line flags; 'has_default' turns a parameter into a command-line flag if and only if it has a default value.")

Plain single quotes (') in the docs to specify strings

Typehint should be Literal['kwonly', 'all', 'has_default']; ditto for bind. (I think I like this order)
@mcarans
Copy link
Author

mcarans commented Nov 7, 2021

@anntzer I've made the changes you suggested. Please take a look.

lib/defopt.py Outdated Show resolved Hide resolved
@anntzer
Copy link
Owner

anntzer commented Nov 7, 2021

Just one typo, then good to go Do you want to take care of squashing your commits yourself or should I do it?

@mcarans
Copy link
Author

mcarans commented Nov 7, 2021

@anntzer I have fixed the double space. I'd be grateful if you could squash the commits.

@anntzer
Copy link
Owner

anntzer commented Nov 7, 2021

Merged into master as bcca151. Thanks for the PR!

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

Successfully merging this pull request may close these issues.

2 participants