Conversation
Mypy already accepts a name of type Any as a base class, but treated args[1] as a generic type application, so *args entries typed as Any were rejected. Index into a known tuple type in base-class position and reuse the existing Any/type/Type[Any] rule without relaxing non-Any bases.
This comment has been minimized.
This comment has been minimized.
|
Can you check this additional test case I generated by modifying one of yours to add nesting (added alongside the ones you added) passes as it should? (I don't think that it does in your current version) pyright 1.1.414 says that the following associated standalone snippet is fine and outputs the expected type for the nested unpack (and pyrefly 1.3.1 also agrees it typechecks): Thanks |
Flatten Unpack[Tuple[...]] before reading a literal index so *tuple[int, *tuple[Any]] is treated like tuple[int, Any].
|
The nested case failed because unpack-in-tuple short-circuited indexing. Nested |
This comment has been minimized.
This comment has been minimized.
|
Thanks so much @00200200 for ensuring your change supports nested unpacked tuples! However the specification also states that unpacking unbounded type var tuples should be generally supported, and they are supported by at your latest commit e0228a2 the above appears to fail with: [edited once to avoid a duplicate function name which did not affect the ability of the unit test to demonstrate the inability of the current version to handle the syntax; did not meaningfully affect the output either since the 2nd function definition was not reached before a failure was produced] You may see relevant examples of this syntax at e.g. https://peps.python.org/pep-0646/#unpacking-unbounded-tuple-types or https://web.archive.org/web/20260918002706/https://typing.python.org/en/latest/spec/generics.html#:~:text=Unpacking%20tuple%20types%20also%20allows%20more%20precise%20types . Thanks! |
|
(I edited my last comment once to avoid a duplicate function name in the proposed additional unit test which did not affect the ability of the unit test to demonstrate the inability of the current version to handle the nested unpacked unbounded tuple syntax; that did not meaningfully affect the output either since the 2nd function definition was not reached before a failure was produced) |
flatten_nested_tuples only removes fixed-length unpacks, so a variadic item still made indexed_tuple_item_type bail out entirely. An index that stays on one side of the variadic item is unambiguous, though: counted from the left before it, or from the right after it. Resolve those and keep returning None for indexes that could land inside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Good catch, thanks — that case did fail, and for a reason worth spelling out.
So instead of rejecting any unpack, the index now resolves when it lands strictly before or strictly after the variadic item, and still returns Full |
This comment has been minimized.
This comment has been minimized.
willy-b
left a comment
There was a problem hiding this comment.
Thanks for your last update ( 99be499 ) to better match the specification by supporting indexing on either side of a nested unpacked unbounded tuple type (see the first example https://peps.python.org/pep-0646/#unpacking-unbounded-tuple-types ).
I think that in the cases of indexing into an unpacked unbounded homogeneous type tuple that are supported by reveal_type (e.g. https://mypy-play.net/?gist=586d52d5406c0e7fcf5054c33e7bfe35 ) you probably want to support those as well (see the same specification, https://peps.python.org/pep-0646/#unpacking-unbounded-tuple-types , just the examples after the first one, or e.g. https://web.archive.org/web/20260918002706/https://typing.python.org/en/latest/spec/generics.html#:~:text=Using%20an%20unpacked%20unbounded%20tuple%20is%20equivalent ).
Note that are some unambiguous cases not yet supported by reveal_type, I think it is not reasonable to expect you to handle those, I may ask about those separately at the reveal_type level in a new issue (see e.g. f3 in https://mypy-play.net/?gist=0ce2970c7a6472603d696e130beeabcc ).
| # A variadic item leaves the length unknown, so only an index that stays on | ||
| # the same side of it resolves: counted from the left before the unpack, or | ||
| # from the right after it. Anything that could land inside it is ambiguous. | ||
| if 0 <= i < unpack: | ||
| return get_proper_type(items[i]) | ||
| if -(n - unpack - 1) <= i < 0: | ||
| return get_proper_type(items[n + i]) |
There was a problem hiding this comment.
I agree that if an unpacked unbounded tuple has types on both sides of it then indexing across that unpacked tuple of unknown length is ambiguous. However, if the nested unpacked unbounded tuple is the only element (e.g. see https://mypy-play.net/?gist=586d52d5406c0e7fcf5054c33e7bfe35 ) or if the type of *args is itself an unpacked unbounded homogeneous type tuple (e.g. in the spec https://web.archive.org/web/20260918002706/https://typing.python.org/en/latest/spec/generics.html#:~:text=Using%20an%20unpacked%20unbounded%20tuple%20is%20equivalent ), then it is not ambiguous whether the index is within it, see various examples after the first at the spec at https://peps.python.org/pep-0646/#unpacking-unbounded-tuple-types
(and there is another case I mention at the end of this comment).
(And unpacked unbounded tuples of homogeneous types are not ambiguous regarding their type e.g. unbounded tuples of Any are known to be Any and should therefore be treated the same as other known Any types.)
Since your first commit in this PR, you treat Any in the case of args[1] with *args: *tuple[int, Any] , as a valid inheritance base, and in your previous commit you would treat args[2] with *args: *tuple[int, *tuple[float, Any]] as a valid base, so args[0] (or args[1]) with *args: *tuple[Any, ...] or *args: *tuple[*tuple[Any, ...]] should also be valid (there is no question in that case whether a given index is within the unpacked nested unbounded tuple or not and what its type is).
Here is an example test case showing a difference between the behavior of your latest commit and reveal_type if your commit intends to allow Anyto be a valid inheritance base when reveal_type would show it as such.
[case testSubclassAnyFromNestedUnpackedUnboundedStarArgsIndexInsideUnambiguous]
# compare https://mypy-play.net/?gist=586d52d5406c0e7fcf5054c33e7bfe35
# https://github.com/python/mypy/issues/21998
# flags: --python-version 3.12
from typing import Any
def from_unpacked(*args: *tuple[Any, ...]) -> None:
reveal_type(args[0]) # N: Revealed type is "Any"
reveal_type(args[1]) # N: Revealed type is "Any"
class Sub(args[0]):
pass
class Sub2(args[1]):
pass
def from_unpacked2(*args: *tuple[*tuple[Any, ...]]) -> None:
reveal_type(args[0]) # N: Revealed type is "Any"
reveal_type(args[1]) # N: Revealed type is "Any"
class Sub(args[0]):
pass
class Sub2(args[1]):
pass
def from_unpacked_type_annotation(*args: *tuple[Any, ...]) -> None:
reveal_type(args[0]) # N: Revealed type is "Any"
reveal_type(args[1]) # N: Revealed type is "Any"
x: type = args[0]
class Sub(x):
pass
x1: type = args[1]
class Sub2(x1):
pass
def from_unpacked2_type_annotation(*args: *tuple[*tuple[Any, ...]]) -> None:
reveal_type(args[0]) # N: Revealed type is "Any"
reveal_type(args[1]) # N: Revealed type is "Any"
x: type = args[0]
class Sub(x):
pass
x1: type = args[1]
class Sub2(x1):
pass
[builtins fixtures/tuple.pyi]
currently fails with:
=================================== FAILURES ===================================
___ testSubclassAnyFromNestedUnpackedUnboundedStarArgsIndexInsideUnambiguous ___
data: /home/liveuser/3rd/mypy/test-data/unit/check-classes.test:9857:
Failed: Unexpected type checker output (/home/liveuser/3rd/mypy/test-data/unit/check-classes.test, line 9857)
----------------------------- Captured stderr call -----------------------------
Expected:
main:8: note: Revealed type is "Any"
main:9: note: Revealed type is "Any"
main:16: note: Revealed type is "Any"
main:17: note: Revealed type is "Any"
main:24: note: Revealed type is "Any"
main:25: note: Revealed type is "Any"
main:34: note: Revealed type is "Any"
main:35: note: Revealed type is "Any"
Actual:
main:8: note: Revealed type is "Any"
main:9: note: Revealed type is "Any"
main:16: note: Revealed type is "Any"
main:17: note: Revealed type is "Any"
main:18: error: Variable "args" is not valid as a type (diff)
main:18: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases (diff)
main:18: error: Invalid base class "args" (diff)
main:18: error: Invalid type: try using Literal[0] instead? (diff)
main:20: error: Variable "args" is not valid as a type (diff)
main:20: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases (diff)
main:20: error: Invalid base class "args" (diff)
main:20: error: Invalid type: try using Literal[1] instead? (diff)
main:24: note: Revealed type is "Any"
main:25: note: Revealed type is "Any"
main:34: note: Revealed type is "Any"
main:35: note: Revealed type is "Any"
Alignment of first line difference:
E: main:24: note: Revealed type is "Any"
A: main:18: error: Variable "args" is not valid as a type
^
Update the test output using --update-data (implies -n0; you can additionally use the -k selector to update only specific tests)
=========================== short test summary info ============================
FAILED mypy/test/testcheck.py::TypeCheckSuite::check-classes.test::testSubclassAnyFromNestedUnpackedUnboundedStarArgsIndexInsideUnambiguous
===================== 1 failed, 14787 deselected in 0.64s ======================
It may be a separate issue whether args[1] for *args: *tuple[int, *tuple[Any, ...]] should be handled by your PR as a valid base though the index unambiguously falls into such a unpacked unbounded homogeneous type tuple (when there is nothing on the other side of it), and it is unambiguously Any typed those cases are not handled by reveal_type yet either (see e.g. https://mypy-play.net/?gist=0ce2970c7a6472603d696e130beeabcc ). So I may report the 3rd case of https://mypy-play.net/?gist=0ce2970c7a6472603d696e130beeabcc as a separate issue.
There was a problem hiding this comment.
This has been mostly resolved by you since I posted it in 14b3f9c (there is now code handling the cases I mentioned put above this part) except for part that is either broken or intentionally not handled already by reveal_type filed at #22018 for team feedback.
There is a bug where GH is not letting me resolve this version of the comment which was on an earlier version of the code and looks out of place here now:
|
Pushed 14b3f9c for the homogeneous unbounded tuple case. |
| if 0 <= i < n: | ||
| return get_proper_type(items[i]) | ||
| return None | ||
| if n == 1: |
There was a problem hiding this comment.
Note that special casing n==1 may be an approach that needs to be immediately updated pending MyPy team feedback on #22018
This comment has been minimized.
This comment has been minimized.
| if self.allow_type_any and t.args: | ||
| item = self.indexed_tuple_item_type(typ, t.args) |
There was a problem hiding this comment.
Note that t.args might be some other Sequence type, not just a tuple here.
E.g. in the case of the following which passes other type checkers, e.g. pyright:
from typing import Any, Sequence
def f(y: Sequence[Any]):
class A(y[0]): pass
At this point you are assuming if it is indexed it will be a tuple type, but it might be some other kind of sequence or just typed as a Sequence:
> /home/liveuser/3rd-party-for-review/mypy/mypy/typeanal.py(1019)analyze_unbound_type_without_type_info()
-> if self.allow_type_any and t.args:
(Pdb) l
1014 breakpoint()
1015 typ = get_proper_type(sym.node.type)
1016 runtime_type = self.anal_type_from_runtime_value(typ)
1017 if runtime_type is not None:
1018 return runtime_type
1019 -> if self.allow_type_any and t.args:
1020 item = self.indexed_tuple_item_type(typ, t.args)
1021 runtime_type = self.anal_type_from_runtime_value(item)
1022 if runtime_type is not None:
1023 return runtime_type
1024 # Option 2:
(Pdb) p t.args
(0,)
(Pdb) p typ.type.fullname
'typing.Sequence'
Then downstream you may be missing the chance to also cover that case supported by pyright by assuming a narrower type than necessary:
(Pdb) next
> /home/liveuser/3rd-party-for-review/mypy/mypy/typeanal.py(1150)indexed_tuple_item_type()
-> if isinstance(typ, Instance) and typ.type.fullname in TUPLE_NAMES and typ.args:
(Pdb) p typ.type.fullname
'typing.Sequence'
| def indexed_tuple_item_type( | ||
| self, typ: ProperType | None, args: tuple[Type, ...] | ||
| ) -> ProperType | None: | ||
| """If this looks like indexing a tuple-typed value, return the item type.""" | ||
| if typ is None or len(args) != 1: | ||
| return None | ||
| if isinstance(typ, Instance) and typ.type.fullname in TUPLE_NAMES and typ.args: | ||
| # Homogeneous tuple[T, ...]: any index has type T. | ||
| return get_proper_type(typ.args[0]) | ||
| if isinstance(typ, TupleType): |
There was a problem hiding this comment.
You are calling indexed_tuple_item_type whenever t.args for UnboundType t is populated, but t.args might not be a tuple for cases that are still valid per other type checkers like Pyright, so you may want to cover all Sequences instead (still only doing special unpack handling where appropriate, I think in tuples).
E.g. the following example passes pyright 1.1.414 :
[case testSubclassFromSequenceIndexed]
# flags: --python-version 3.12
from typing import Any, Sequence
def f(y: Sequence[Any]):
class A(y[0]): pass
but would not pass your code; it makes it into this indexed_tuple_item_type but is not recognized as something that can be indexed into because you may have been too specific:
(Pdb) next
> /home/liveuser/3rd-party-for-review/mypy/mypy/typeanal.py(1150)indexed_tuple_item_type()
-> if isinstance(typ, Instance) and typ.type.fullname in TUPLE_NAMES and typ.args:
(Pdb) p typ.type.fullname
'typing.Sequence'
I am leaving this comment so you can consider whether you want to support such cases which are supported by Pyright by updating your code to handle Sequence, not just tuple here, thanks!
| # A variadic item leaves the length unknown, so only an index that stays on | ||
| # the same side of it resolves: counted from the left before the unpack, or | ||
| # from the right after it. Anything that could land inside it is ambiguous. | ||
| if 0 <= i < unpack: | ||
| return get_proper_type(items[i]) | ||
| if -(n - unpack - 1) <= i < 0: | ||
| return get_proper_type(items[n + i]) |
There was a problem hiding this comment.
This has been mostly resolved by you since I posted it in 14b3f9c (there is now code handling the cases I mentioned put above this part) except for part that is either broken or intentionally not handled already by reveal_type filed at #22018 for team feedback.
There is a bug where GH is not letting me resolve this version of the comment which was on an earlier version of the code and looks out of place here now:
When an indexed variable is typed as a Sequence (e.g. y: Sequence[Any]), resolve the item type by mapping the instance to typing.Sequence so indexing matches other sequences in base class position.
|
Pushed |
for more information, see https://pre-commit.ci
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
There is no need for this anymore (also it wasn't a good fix anyway). |
|
Actually there was some confusion, I thought this fixes another issue. But after looking at the right issue, I still think this is a wrong approach. |
Fixes #21998
class C(args[1])failed even when that slot isAny. Names of typeAnyalready work as dynamic bases; tuple indexes didn't. I allowAny/type/Type[Any]indexes and still reject everything else.x = args[1]; class C(x)still needs an annotation — bases are resolved before inference.Tested with the new cases in
check-classes.test(pytest mypy/test/testcheck.py).