Skip to content

Fix _deprecate_positional_args for kwonly args w/o default#16850

Merged
NicolasHug merged 2 commits intoscikit-learn:masterfrom
thomasjpfan:fix_deprecated_position_args
Apr 14, 2020
Merged

Fix _deprecate_positional_args for kwonly args w/o default#16850
NicolasHug merged 2 commits intoscikit-learn:masterfrom
thomasjpfan:fix_deprecated_position_args

Conversation

@thomasjpfan
Copy link
Copy Markdown
Member

Fixes _deprecate_positional_args for keyword only arguments w/o defaults arguments i.e.:

@_deprecate_positional_args
def hello(a, *, b, c=3):
	pass

CC @adrinjalali

Copy link
Copy Markdown
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Thanks @thomasjpfan

@adrinjalali
Copy link
Copy Markdown
Member

@NicolasHug should be an easy review.

Copy link
Copy Markdown
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +1101 to +1102
def f3(a, *, b, c=1, d=1):
pass
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

small comment?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added small comment above function.

@cmarmo
Copy link
Copy Markdown
Contributor

cmarmo commented Apr 14, 2020

@scikit-learn/core-devs , time to merge this one? :)

@NicolasHug NicolasHug changed the title ENH Fixes _deprecate_positional_args for keyword only arguments w/o defaults Fix _deprecate_positional_args for kwonly args w/o default Apr 14, 2020
@NicolasHug NicolasHug merged commit 9901d8d into scikit-learn:master Apr 14, 2020
gio8tisu pushed a commit to gio8tisu/scikit-learn that referenced this pull request May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants