[ty] Homogeneous and mixed tuples #18600
Merged
Conversation
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
|
|
This is really exciting! One nit:
I thought we already supported homogenous tuples following #17998 — I thought it was just the mixed partially-homogenous-partially-heterogeneous ones that we didn't support yet? |
Yep you're right! I had just put in a placeholder PR body for now |
* main: [`pylint`] De-emphasize `__hash__ = Parent.__hash__` (`PLW1641`) (#18613) [`flake8-pyi`] Avoid syntax error in the case of starred and keyword arguments (`PYI059`) (#18611) [ty] Add support for global __debug__ constant (#18540) [`ruff`] Preserve parentheses around `deque` in fix for `unnecessary-empty-iterable-within-deque-call` (`RUF037`) (#18598) [`refurb`] Parenthesize lambda and ternary expressions in iter (`FURB122`, `FURB142`) (#18592)
CodSpeed Instrumentation Performance ReportMerging #18600 will degrade performances by 11.29%Comparing Summary
Benchmarks breakdown
|
dcreager
commented
Jun 10, 2025
* main: [ty] Add some "inside string" tests for `object.<CURSOR>` completions [ty] Pull types on synthesized Python files created by mdtest (#18539) Update Rust crate anstyle to v1.0.11 (#18583) [`pyupgrade`] Fix `super(__class__, self)` detection in UP008 (super-call-with-parameters) (#18478) [ty] Generate the top and bottom materialization of a type (#18594) `SourceOrderVisitor` should visit the `Identifier` part of the `PatternKeyword` node (#18635) Update salsa (#18636) [ty] Update mypy_primer doc (#18638) [ty] Improve support for `object.<CURSOR>` completions [ty] Add `CoveringNode::find_last` [ty] Refactor covering node representation [ty] Infer the Python version from `--python=<system installation>` on Unix (#18550) [`flake8-return`] Fix `RET504` autofix generating a syntax error (#18428) Fix incorrect salsa `return_ref` attribute (#18605) Move corpus tests to `ty_python_semantic` (#18609) [`pyupgrade`] Don't offer fix for `Optional[None]` in non-pep604-annotation-optional (`UP045)` or non-pep604-annotation-union (`UP007`) (#18545) [`pep8-naming`] Suppress fix for `N804` and `N805` if the recommend name is already used (#18472) [`ruff`] skip fix for `RUF059` if dummy name is already bound (unused-unpacked-variable) (#18509)
|
(It's fine to address this comment in a followup; doesn't need to be tackled in this PR!) -- Do we need to add some more tests and TODOs around generics solving for mixed tuples and pure-homogeneous tuples? It looks like for these four from typing import reveal_type
def f[T](x: tuple[int, bytes, *tuple[str, ...], T, int]) -> T:
return x[-2]
reveal_type(f((1, b"foo", "bar", "baz", True, 42))) # Unknown
def f2[T](x: tuple[int, T, *tuple[str, ...], bool, int]) -> T:
return x[1]
reveal_type(f2((1, b"foo", "bar", "baz", True, 42))) # Unknown
def g[T](x: tuple[T, int]) -> T:
return x[0]
reveal_type(g((True, 42))) # bool
def h[T](x: tuple[T, ...]) -> T:
return x[0]
reveal_type(h((42,))) # Unknown |
AlexWaygood
reviewed
Jun 20, 2025
crates/ty_python_semantic/resources/mdtest/type_properties/is_assignable_to.md
Outdated
Show resolved
Hide resolved
I misunderstood this the first time around — I thought you were asking about |
AlexWaygood
approved these changes
Jun 20, 2025
Done. (We already had a TODO comment in the code, just no TODO tests) |
CodSpeed WallTime Performance ReportMerging #18600 will improve performances by 4.04%Comparing Summary
Benchmarks breakdown
|
|
There is a fairly big primer report here -- have you spot-checked them/do the new diagnostics make sense? |
|
Another really useful follow-up to uncover any issues here would be to add support to our property tests to generate variadic and mixed tuple types. Should be pretty easy (unless it uncovers actual problems!) |
I uploaded a rich diff here: https://shark.fish/diff-tuple-spec.html |
I have a WIP patch locally to fix the 58 new |
This one is because there's a function that returns a |
|
A lot of the new |
dcreager
added a commit
that referenced
this pull request
Jun 24, 2025
Add property test generators for the new variable-length tuples. This covers homogeneous tuples as well. The property tests did their job! This identified several fixes we needed to make to various type property methods. cf #18600 (comment) --------- Co-authored-by: Alex Waygood <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We already had support for homogeneous tuples (
tuple[int, ...]). This PR extends this to also support mixed tuples (tuple[str, str, *tuple[int, ...], str str]).A mixed tuple consists of a fixed-length (possibly empty) prefix and suffix, and a variable-length portion in the middle. Every element of the variable-length portion must be of the same type. A homogeneous tuple is then just a mixed tuple with an empty prefix and suffix.
The new data representation uses different Rust types for a fixed-length (aka heterogeneous) tuple. Another option would have been to use the
VariableLengthTuplerepresentation for all tuples, and to wrap the "variable + suffix" portion in anOption. I don't think that would simplify the method implementations much, though, since we would still have a 2×2 case analysis for most of them.One wrinkle is that the definition of the
tupleclass in the typeshed has a single typevar, and canonically represents a homogeneous tuple. When getting the class of a tuple instance, that means that we have to summarize our detailed mixed tuple type information into its "homogeneous supertype". (We were already doing this for heterogeneous types.)A similar thing happens when concatenating two mixed tuples: the variable-length portion and suffix of the LHS, and the prefix and variable-length portion of the RHS, all get unioned into the variable-length portion of the result. The LHS prefix and RHS suffix carry through unchanged.