-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Don't consider bound self
parameter when comparing bound methods
#20692
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
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
CodSpeed WallTime Performance ReportMerging #20692 will improve performances by 10.87%Comparing Summary
Benchmarks breakdown
|
|
Lint rule | Added | Removed | Changed |
---|---|---|---|
invalid-argument-type |
17 | 1 | 1 |
invalid-assignment |
3 | 0 | 14 |
not-iterable |
3 | 0 | 1 |
invalid-return-type |
1 | 1 | 0 |
unused-ignore-comment |
1 | 1 | 0 |
invalid-key |
1 | 0 | 0 |
no-matching-overload |
0 | 1 | 0 |
possibly-missing-attribute |
0 | 0 | 1 |
unresolved-attribute |
1 | 0 | 0 |
Total | 27 | 4 | 17 |
CodSpeed Instrumentation Performance ReportMerging #20692 will improve performances by 8.68%Comparing Summary
Benchmarks breakdown
Footnotes
|
@AlexWaygood convinced me synchronously that this is wrong, and so is astral-sh/ty#1301! The bound methods of two different classes really are different sets of objects, and are correctly not subtypes of each other. It was a different issue that I was encountering on #20677. |
In a bound method, the bound
self
parameter is no longer available for callers to provide an argument for. (That's the whole point of binding it!) That means we should not consider it when checking subtyping/assignability of bound methods.We were already doing this correctly when comparing a bound method to other callable types, since we would coerce the bound method into its equivalent
Callable
type before performing the check, and our conversion into aCallable
type correctly removes the boundself
parameter from the signature.But we weren't doing the same when comparing a bound method to another bound method — instead we were comparing the unbound function signatures, and also comparing the bound
self
instance types. We can get the correct behavior (and eliminate having to duplicate logic) by coercing both bound methods intoCallable
types for these checks, too.Closes astral-sh/ty#1301