Skip to content

[API] Rename py::kwonly to py::kw_only #2410

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

Closed
henryiii opened this issue Aug 19, 2020 · 7 comments · Fixed by #2459
Closed

[API] Rename py::kwonly to py::kw_only #2410

henryiii opened this issue Aug 19, 2020 · 7 comments · Fixed by #2459
Assignees
Labels
policy Suggestions for changes in policy

Comments

@henryiii
Copy link
Collaborator

henryiii commented Aug 19, 2020

The py::kwonly() feature is great, but the naming is not very consistent, I believe. It has not been in a released version of pybind11 yet, so I think we can safely change it to py::kw_only()? Would that be a good idea? (see, for example, py::is_final(), which was also recently added, or py::module_local(), py::buffer_protocol(), etc.).

@YannickJadoul
Copy link
Collaborator

I assume kwonly was meant to match kwargs?
A quick search shows that both are used, kw_only notably by attrs, but kwonlyargs by Python itself in the inspect module.

More important to me: I did notice the signature doesn't contain a *. Should we try to fix that? (happy to make a PR)

@henryiii
Copy link
Collaborator Author

The signature should, yes. I'd like to have a pos_only and / from Python 3.8 too, honestly. Don't think it would be a hard addition.

@henryiii
Copy link
Collaborator Author

It could have been done that way; if there's a good reason, we can keep it that way. That's why I've assigned @wjakob , since it's an API choice. **kwargs is a common, making that a common thing to type in Python, but I think py::kw_only() is more like the other pybind11 tags. But would be happy with either choice as long as it's a conscious decision.

@henryiii henryiii added the policy Suggestions for changes in policy label Aug 26, 2020
@wjakob
Copy link
Member

wjakob commented Sep 2, 2020

I very slightly lean towards kw_only given the consistency with the other tags @henryiii mentioned, but I don't have a strong opinion at all. If anyone champions the current naming then that would be fine as well. We could vote? :)

@skoslowski @jagerman

@skoslowski
Copy link
Contributor

I prefer kw_only, mostly for consistency. Especially considering a potential pos_only flag - posonly would be really hard to read...

@jagerman
Copy link
Member

jagerman commented Sep 3, 2020

Weak preference for kw_only over kwonly

@henryiii
Copy link
Collaborator Author

henryiii commented Sep 3, 2020

Seems like py:kw_only is winning. I've added it to the PR above, but it is a separate comment so still could be dropped easily.

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

Successfully merging a pull request may close this issue.

5 participants