-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
bpo-37540: vectorcall: keyword names must be strings #14682
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
Conversation
Instead of "string", I'd say "`str` or a subclass" explicitly. |
There are many instances in the documentation (unrelated to vectorcall) where the word "string" is used. I don't see the problem here. Concrete proposal: we leave this doc as is and I will reboot #13844 (I always planned to do that, but after all other PRs that affect documentation). In the section describing the "vectorcall protocol", I will say explicitly what "string" means for vectorcall. |
Is this PR okay now or do you want me to change things? |
@markshannon, you said you were looking at this PR. Is that still the case? Any progress? |
Ping? |
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.
@markshannon seems unresponsive. I don't see an issue with this PR.
@@ -256,7 +256,7 @@ | |||
>>> f(**{1: 3}, **{1: 5}) | |||
Traceback (most recent call last): | |||
... | |||
TypeError: f() keywords must be strings |
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.
I see f(**{1:2})
is still tested in test_excall.py.
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.
Indeed. This is testing two errors at the same time (duplicate keyword and non-string keyword). Which of the two errors you get seems arbitrary to me and this PR changes the error.
Thanks! This will help to make progress on some other PRs. |
The fact that keyword names are strings is now part of the vectorcall and `METH_FASTCALL` protocols. The biggest concrete change is that `_PyStack_UnpackDict` now checks that and raises `TypeError` if not. CC @markshannon @vstinner https://bugs.python.org/issue37540
The fact that keyword names are strings is now part of the vectorcall and `METH_FASTCALL` protocols. The biggest concrete change is that `_PyStack_UnpackDict` now checks that and raises `TypeError` if not. CC @markshannon @vstinner https://bugs.python.org/issue37540
The fact that keyword names are strings is now part of the vectorcall and `METH_FASTCALL` protocols. The biggest concrete change is that `_PyStack_UnpackDict` now checks that and raises `TypeError` if not. CC @markshannon @vstinner https://bugs.python.org/issue37540
The fact that keyword names are strings is now part of the vectorcall and
METH_FASTCALL
protocols. The biggest concrete change is that_PyStack_UnpackDict
now checks that and raisesTypeError
if not.CC @markshannon @vstinner
https://bugs.python.org/issue37540
Automerge-Triggered-By: @encukou