[ty] Avoid literal promotion for covariant declared types#23186
Open
ibraheemdev wants to merge 1 commit intomainfrom
Open
[ty] Avoid literal promotion for covariant declared types#23186ibraheemdev wants to merge 1 commit intomainfrom
ibraheemdev wants to merge 1 commit intomainfrom
Conversation
ibraheemdev
requested review from
AlexWaygood,
carljm and
sharkdp
as code owners
Typing conformance resultsNo changes detected ✅ |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
13 | 14 | 111 |
type-assertion-failure |
9 | 3 | 18 |
unsupported-operator |
4 | 4 | 21 |
possibly-missing-attribute |
1 | 15 | 12 |
unresolved-attribute |
15 | 0 | 0 |
invalid-assignment |
0 | 4 | 7 |
no-matching-overload |
0 | 11 | 0 |
unused-type-ignore-comment |
10 | 1 | 0 |
unsupported-base |
4 | 0 | 0 |
invalid-key |
0 | 0 | 1 |
invalid-return-type |
1 | 0 | 0 |
possibly-unresolved-reference |
0 | 1 | 0 |
| Total | 57 | 53 | 170 |
ibraheemdev
force-pushed
the
ibraheem/covariant-literal-promotion
branch
from
0a8d74a to
46aac79
Compare
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
|
Member
|
I think the ecosystem diff looks good overall, but it's a bit hard to tell because of the number of flakes. If you rebase on top of dc842db, you should get a cleaner ecosystem report that's easier to review. Here's a few interesting added diagnostics I spotted for now:
This one is a false positive, but I think it's due to an overly restrictive type annotation in typeshed for
This one is also a false positive, but I think it could be fixed in the future with the more advanced bidirectional inference we have planned. It's basically the same issue that we'll have with something like this, as soon as we get rid of our def f() -> list[int | None]:
x = [1, 2, 3]
return x
This one seems a bit problematic. Do we need to recurse into nested collection literals when deciding that that element-types inside those literals shouldn't be promoted? |
ibraheemdev
force-pushed
the
ibraheem/covariant-literal-promotion
branch
from
46aac79 to
43d50fa
Compare
Member
Author
Hmm, I'm not sure recursive promotion makes sense there, because the inner set elements are not actually in covariant position, and the set itself is still an invariant collection accessible through the list. I think this is a known limitation of the constraint solver not being able to solve unions of typevars: from typing import Sequence, Iterable, Literal
def check_sequence_matches[T](seq: Sequence[T], template: Iterable[T | set[T]]):
...
def _(x: list[Literal[1]], y: list[Literal[1] | set[int]]):
check_sequence_matches(x, y) # error: Expected `Iterable[Literal[1] | set[Literal[1]]]`, found `list[Literal[1] | set[int]]`For reference, pyright does not error here. |
ibraheemdev
force-pushed
the
ibraheem/covariant-literal-promotion
branch
from
43d50fa to
3bf92a5
Compare
Memory usage reportSummary
Significant changesClick to expand detailed breakdownflake8
prefect
sphinx
trio
|
Contributor
|
It seems like this might not be entirely desirable until we do #136? It will cause an example like this to error: from typing import Iterable
x: Iterable[int] = [1, 2]
reveal_type(x)
x.append(4) # bad argument, expected `Literal[1, 2]`Of course that example will also error after #136, but for a more intuitive reason. But with #136, you have the option to opt-out of the upcast by separating declaration from assignment: from typing import Iterable
x: Iterable[int]
x = [1, 2]
reveal_type(x)
x.append(4)It seems like with this PR, that will continue to error, which doesn't seem desirable -- I think we'd want literal promotion there. It's not clear to me that the declared type being covariant is the right heuristic here, since an inferred type can always be a subtype of the declared type, so the inferred type being invariant can still cause problems with overly-precise inference. |
carljm
requested changes
Feb 14, 2026
Contributor
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
Requesting changes to move this out of my review queue pending discussion of my comment above.
Member
Author
|
I didn't realize that your second example would prefer the inferred type there. Maybe we need a more precise heuristic here based on the "source" of the type context? e.g., |
Contributor
|
Basically we want this logic only where the inferred type is effectively going to disappear in favor of the declared type. That's true for a function argument/parameter, and it's true for a direct annotated assignment (after #136). But if that's the desired behavior, then do we need this at all? Or do we just need #136? If the inferred type isn't going to be used, does it matter how we infer it (as long as we correctly infer assignability to the declared type). |
Member
Author
|
I think it still matters for generic inference, e.g., if the parameter type is The loop and for comprehension case is also interesting, because there isn't really a declared type there, it's more akin to generic inference against an |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Avoid literal promotion (and
Unknownwidening) for elements in covariant position in their declared type, e.g.,Resolves astral-sh/ty#2280, resolves astral-sh/ty#2441.