-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Replace positional arguments with keyword arguments #1664
Replace positional arguments with keyword arguments #1664
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! I like this move to keyword arguments. Complex interfaces are more intuitive now.
Apologies for bumping an old PR - thought it better than opening a new issue about this.
This is true in most cases - however, it doesn't make sense in all, and the "one-size-fits-all" approach that's been applied here could be improved, I think. For instance:
There's no reason for the The move to kwargs is a sensible one overall, but I think there might be some design decisions worth revisiting in a future update. I defer to your much greater experience building Faker than mine, though! |
Came here to say this as well. |
I agree here as well. |
Yeah! We should probably centralize this discussion in an issue. |
Have created #1692 to discuss this. |
* Replace positional arguments with keyword arguments * Update CHANGELOG entries * Update docs
* Replace positional arguments with keyword arguments * Update CHANGELOG entries * Update docs
Issue
This PR is related to #1356 and #1651.
Description
Replacing positional arguments with keywords arguments.