[ty] Fix assignability of intersections with bounded typevars#24193
[ty] Fix assignability of intersections with bounded typevars#24193
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 86.46%. The percentage of expected errors that received a diagnostic held steady at 80.68%. The number of fully passing files held steady at 67/132. |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownsphinx
prefect
trio
flake8
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
40 | 0 | 0 |
invalid-argument-type |
0 | 11 | 0 |
invalid-return-type |
1 | 0 | 0 |
| Total | 41 | 11 | 0 |
Large timing changes:
| Project | Old Time | New Time | Change |
|---|---|---|---|
egglog-python |
0.49s | 1.56s | +216% |
pandera |
0.77s | 1.87s | +143% |
mitmproxy |
0.34s | 0.51s | +52% |
Changes in flaky projects detected. Raw diff output excludes flaky projects; see the HTML report for details.
Raw diff:
Tanjun (https://github.com/FasterSpeeding/Tanjun)
- tanjun/annotations.py:1949:74 error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `int | float`, found `_NumberT@__getitem__ & ~int`
- tanjun/annotations.py:1996:74 error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `int | float`, found `_NumberT@__getitem__ & ~int`
- tanjun/annotations.py:2110:75 error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `int | float`, found `_NumberT@__getitem__ & ~float`
- tanjun/annotations.py:2110:86 error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `int | float`, found `_NumberT@__getitem__ & ~float`
- tanjun/annotations.py:2724:26 error[invalid-argument-type] Argument to function `parse_annotated_args` is incorrect: Expected `SlashCommand[Any] | MessageCommand[Any]`, found `_CommandUnionT@with_annotated_args & ~AlwaysFalsy`
altair (https://github.com/vega/altair)
- altair/vegalite/v6/api.py:1179:45 error[invalid-argument-type] Argument to function `_reveal_parsed_shorthand` is incorrect: Expected `Mapping[str, Any]`, found `_C@Then & Top[dict[Unknown, Unknown]]`
artigraph (https://github.com/artigraph/artigraph)
- src/arti/internal/type_hints.py:177:37 error[invalid-argument-type] Argument to function `_check_issubclass` is incorrect: Expected `type`, found `T@lenient_issubclass & ~tuple[object, ...]`
beartype (https://github.com/beartype/beartype)
- beartype/_decor/_nontype/decornontype.py:161:43 error[invalid-argument-type] Argument is incorrect: Argument type `BeartypeableT@beartype_nontype & ~type` does not satisfy upper bound `((...) -> Any) | classmethod[Unknown, (...), Unknown] | property` of type variable `BeartypeableT`
- beartype/_decor/_nontype/decornontype.py:220:47 error[invalid-argument-type] Argument is incorrect: Argument type `BeartypeableT@beartype_nontype & ~type` does not satisfy upper bound `((...) -> Any) | classmethod[Unknown, (...), Unknown] | property` of type variable `BeartypeableT`
- beartype/_decor/decorcore.py:133:26 error[invalid-argument-type] Argument to function `beartype_nontype` is incorrect: Argument type `BeartypeableT@_beartype_object_fatal & ~type` does not satisfy upper bound `((...) -> Any) | classmethod[Unknown, (...), Unknown] | property` of type variable `BeartypeableT`
setuptools (https://github.com/pypa/setuptools)
- setuptools/glob.py:75:42 error[invalid-argument-type] Argument to function `has_magic` is incorrect: Expected `str | bytes`, found `AnyStr@_iglob & ~AlwaysFalsy`
Merging this PR will degrade performance by 83.24%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | ty_micro[large_union_narrowing] |
498.2 ms | 2,973.5 ms | -83.24% |
| ❌ | WallTime | pandas |
69.6 s | 72.7 s | -4.26% |
| ❌ | WallTime | altair |
5.1 s | 5.7 s | -9.91% |
| ❌ | WallTime | colour_science |
72.5 s | 77.7 s | -6.69% |
| ❌ | WallTime | multithreaded |
1.3 s | 1.5 s | -14.04% |
| ❌ | Simulation | hydra-zen |
1.1 s | 1.2 s | -6.32% |
Comparing cjm/tvinter (04da369) with main (55c5d90)
Footnotes
-
30 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
Looks like this needs some perf work! |
| let expanded = intersection.map_positive(db, |element| match *element { | ||
| Type::TypeVar(typevar) if !typevar.is_inferable(db, self.inferable) => { | ||
| // Leave inferable typevars alone: widening them here could bypass the normal | ||
| // solving path and discard information needed to infer a concrete specialization. | ||
| match typevar.typevar(db).bound_or_constraints(db) { | ||
| Some(TypeVarBoundOrConstraints::UpperBound(bound)) => bound, | ||
| Some(TypeVarBoundOrConstraints::Constraints(constraints)) => { | ||
| constraints.as_type(db) | ||
| } | ||
| None => *element, | ||
| } | ||
| } | ||
| _ => element | ||
| .as_new_type() | ||
| .map_or(*element, |newtype| newtype.concrete_base_type(db)), | ||
| }); | ||
|
|
||
| (expanded != Type::Intersection(intersection)).then_some(expanded) |
There was a problem hiding this comment.
a pre-check may improve performance here. And, I added a helper method for exactly this expansion the other day, which you can use here.
| let expanded = intersection.map_positive(db, |element| match *element { | |
| Type::TypeVar(typevar) if !typevar.is_inferable(db, self.inferable) => { | |
| // Leave inferable typevars alone: widening them here could bypass the normal | |
| // solving path and discard information needed to infer a concrete specialization. | |
| match typevar.typevar(db).bound_or_constraints(db) { | |
| Some(TypeVarBoundOrConstraints::UpperBound(bound)) => bound, | |
| Some(TypeVarBoundOrConstraints::Constraints(constraints)) => { | |
| constraints.as_type(db) | |
| } | |
| None => *element, | |
| } | |
| } | |
| _ => element | |
| .as_new_type() | |
| .map_or(*element, |newtype| newtype.concrete_base_type(db)), | |
| }); | |
| (expanded != Type::Intersection(intersection)).then_some(expanded) | |
| if intersection.positive(db).iter().any(|t| matches!(t, Type::NewTypeInstance(_) | Type::TypeVar(_)) { | |
| Some(intersection.with_expanded_typevars_and_newtypes(db)) | |
| } else { | |
| None | |
| } |
|
Closing in favor of #24502 |
Summary
Fixes astral-sh/ty#3137
We already supported assigning a bounded/constrained typevar to a super-type of its bound / the union of its constraints. But this handling was not applied in case the typevar was part of an intersection; this PR fixes that.
The expansion applies only to non-inferable typevars -- if we applied it to inferable typevars, we'd miss finding their constraints.
There's some code here for
NewTypealso, but it doesn't handle any new cases, just maintains the existing support.Test Plan
Added mdtests.
Non-flaky ecosystem results are all cases of the expected false positives going away.