Skip to content

Comments

Fix instance vs callable subtyping/assignability#18260

Merged
carljm merged 4 commits intomainfrom
cjm/callable-instances
May 22, 2025
Merged

Fix instance vs callable subtyping/assignability#18260
carljm merged 4 commits intomainfrom
cjm/callable-instances

Conversation

@carljm
Copy link
Contributor

@carljm carljm commented May 22, 2025 •
edited
Loading

Summary

Fix some issues with subtying/assignability for instances vs callables. We need to look up dunders on the class, not the instance, and we should limit our logic here to delegating to the type of __call__, so it doesn't get out of sync with the calls we allow.

Also, we were just entirely missing assignability handling for __call__ implemented as anything other than a normal bound method (though we had it for subtyping.)

A first step towards considering what else we want to change in astral-sh/ty#491

Test Plan

mdtests

@github-actions
Copy link
Contributor

github-actions bot commented May 22, 2025

mypy_primer results

Changes were detected when running on open source projects 
strawberry (https://github.com/strawberry-graphql/strawberry)
- error[invalid-assignment] strawberry/federation/scalar.py:133:9: Object of type `_T | None` is not assignable to `((...) -> Unknown) | None`
- error[invalid-argument-type] strawberry/types/scalar.py:110:29: Argument to bound method `__init__` is incorrect: Expected `(Any, /) -> Any`, found `_T`
- error[invalid-assignment] strawberry/types/scalar.py:209:9: Object of type `_T | None` is not assignable to `((...) -> Unknown) | None`
- Found 436 diagnostics
+ Found 433 diagnostics

jinja (https://github.com/pallets/jinja)
- error[invalid-argument-type] src/jinja2/ext.py:168:27: Argument to bound method `call` is incorrect: Expected `(...) -> Any`, found `Any | Undefined`
- Found 234 diagnostics
+ Found 233 diagnostics

@carljm carljm mentioned this pull request May 22, 2025
Closed
@MichaReiser MichaReiser added the ty Multi-file analysis & type inference label May 22, 2025
AlexWaygood
AlexWaygood approved these changes May 22, 2025
View reviewed changes
carljm and others added 3 commits May 22, 2025 15:28
@carljm carljm enabled auto-merge (squash) May 22, 2025 19:44
@carljm carljm merged commit 0b181bc into main May 22, 2025
30 checks passed
@carljm carljm deleted the cjm/callable-instances branch May 22, 2025 19:47
@carljm carljm mentioned this pull request May 27, 2025
Merged
carljm added a commit that referenced this pull request May 27, 2025
@carljm
put similar dunder-call tests next to each other (#18343)
3eada01 
Follow-up from post-land review on
#18260
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sharkdp sharkdp sharkdp left review comments

@AlexWaygood AlexWaygood AlexWaygood approved these changes

@dcreager dcreager Awaiting requested review from dcreager dcreager is a code owner

Assignees

No one assigned

Labels

ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@carljm @sharkdp @AlexWaygood @MichaReiser