-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-127750: Fix annotations in singledispatchmethod signature tests #143571
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
gh-127750: Fix annotations in singledispatchmethod signature tests #143571
Conversation
… the right params See https://github.com/python/cpython/pull/130309/changes#r2663516538. These functions don't annotate parameters for values used in single-dispatching (`item`), they annotate parameters one later (`arg`). This being legal relies on a bug -- pythonGH-84644, which is that `singledispatch` doesn't verify the annotation is on the "first" parameter. I think these functions should look like ```diff @functools.singledispatchmethod - def func(self, item, arg: int) -> str: + def func(self, item: int, arg) -> str: return str(item) @func.register - def _(self, item, arg: bytes) -> str: + def _(self, item: bytes, arg) -> str: return str(item) ``` (and signature tests updated accordingly) In practice, it doesn't matter how you annotate `func` here, because it's a default callback. But what matters is that any function passed to `register()` annotates the target parameter (always first positional in `singledispatch` and always second positional in `singledispatchmethod` (unless staticmethod, then the first) etc.) for the value to single-dispatch on; not some other, unrelated parameter (or return type, as presented in pythonGH-84644). Let's have a class with the same registree signature as these from the test: ```py class A: @functools.singledispatchmethod def func(self, item, arg: int) -> str: return 'argint' @func.register def _(self, item, arg: bytes) -> str: return 'argbytes' a = A() print(a.func(b'', 1)) ``` For this code, the output would be `argbytes`, even though `arg` is an integer `1`.
Co-authored-by: Serhiy Storchaka <[email protected]>
|
@serhiy-storchaka, did I get it correctly? Thanks for the quick review :) |
serhiy-storchaka
left a comment
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.
LGTM. 👍
|
Thanks @johnslavik for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
…sts (pythonGH-143571) These tests relied on a bug -- pythongh-84644, which is that singledispatch doesn't verify the annotation is on the "first" parameter. (cherry picked from commit 620a5b92693ac1b2cef1f90fd3c2dba1bb794552) Co-authored-by: Bartosz Sławecki <[email protected]>
|
GH-143707 is a backport of this pull request to the 3.14 branch. |
…sts (pythonGH-143571) These tests relied on a bug -- pythongh-84644, which is that singledispatch doesn't verify the annotation is on the "first" parameter. (cherry picked from commit 620a5b9) Co-authored-by: Bartosz Sławecki <[email protected]>
|
GH-143708 is a backport of this pull request to the 3.13 branch. |
…ests (GH-143571) (GH-143708) These tests relied on a bug -- gh-84644, which is that singledispatch doesn't verify the annotation is on the "first" parameter. (cherry picked from commit 620a5b9) Co-authored-by: Bartosz Sławecki <[email protected]>
…ests (GH-143571) (GH-143707) These tests relied on a bug -- gh-84644, which is that singledispatch doesn't verify the annotation is on the "first" parameter. (cherry picked from commit 620a5b9) Co-authored-by: Bartosz Sławecki <[email protected]>
Follow-up to gh-130309.
See https://github.com/python/cpython/pull/130309/changes#r2663516538.
Details
These functions don't annotate parameters for values used in single-dispatching (
item), they annotate parameters one later (arg).This being legal relies on a bug -- GH-84644, which is that
singledispatchdoesn't verify the annotation is on the "first" parameter.In practice, it doesn't matter how you annotate
funchere, because it's a default callback. But what matters is that any function passed toregister()annotates the target parameter (always first positional insingledispatchand always second positional insingledispatchmethod(unless staticmethod, then the first) etc.) for the value to single-dispatch on; not some other, unrelated parameter (or return type, as presented in GH-84644).Let's have a class with the same registree signature as these from the test:
For this code, the output would be
argbytes, even thoughargis an integer1.