Skip to content

Keyword only args #35

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 18 commits into from
Apr 12, 2020
Merged

Keyword only args #35

merged 18 commits into from
Apr 12, 2020

Conversation

@aaltat
Copy link
Contributor

@aaltat aaltat commented Apr 4, 2020 •
edited
Loading

Fixes #9

@aaltat aaltat force-pushed the keyword_only_args branch from 2ccc4c0 to 92864e6 Compare April 4, 2020 22:41
@aaltat aaltat force-pushed the keyword_only_args branch from d87e4bc to 994851f Compare April 4, 2020 23:15
@aaltat aaltat requested a review from pekkaklarck April 4, 2020 23:19
pekkaklarck
pekkaklarck requested changes Apr 5, 2020
View reviewed changes
Copy link
Member

@pekkaklarck pekkaklarck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only looked the code and it looks fine except that it totally ignores kwonly args without defaults. An annoying thing with handling defaults in general is that the getfullargspec returns normal args and their possible defaults differently than kwonly args and their possible defaults. On Robot side this is handled so that ArgumentSpec has positional and kwonlyargs which both contain only argument names (i.e. are lists) and then separate defaults that is a mapping from argument name (either normal or kwonly) to their default value. Something like that, including similar ArgumentSpec object, could work here as well. Let's talk more on Slack!

I also added some comments related how the code possibly could be simplified.

@aaltat aaltat merged commit f2c1779 into robotframework:master Apr 12, 2020
@aaltat aaltat deleted the keyword_only_args branch April 12, 2020 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pekkaklarck pekkaklarck pekkaklarck requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support keyword-only arguments

2 participants

@aaltat @pekkaklarck