gh-108494: Argument Clinic: fix option group for Limited C API by vstinner · Pull Request #108574 · python/cpython
Conversation
Use PyTuple_Size() instead of PyTuple_GET_SIZE().
Issue found while converting syslog to the limited C API.
Comment on lines +1718 to +1721
| nargs = 'PyTuple_Size(args)' | ||
| else: | ||
| nargs = 'PyTuple_GET_SIZE(args)' | ||
| add(f"switch ({nargs}) {{\n") |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style suggestion -- because of the f-strings needing the duplication of {, perhaps just being explicit in both cases would be worth it?
| nargs = 'PyTuple_Size(args)' | |
| else: | |
| nargs = 'PyTuple_GET_SIZE(args)' | |
| add(f"switch ({nargs}) {{\n") | |
| add("switch (PyTuple_Size(args)) {\n") | |
| else: | |
| add("switch (PyTuple_GET_SIZE(args)) {\n") |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, other than @AA-Turner's style nit, which I agree with :)
Merged, thanks for reviews. I prefer to avoid repetition even if the f-string is just a little bit surprising.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters