-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[ty] Represent NamedTuple as an opaque special form, not a class
#19915
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-08-15 10:59:09.219027094 +0000
+++ new-output.txt 2025-08-15 10:59:09.287027633 +0000
@@ -1,5 +1,5 @@
WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors.
-fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/918d35d/src/function/execute.rs:215:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(445b)): execute: too many cycle iterations`
+fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/918d35d/src/function/execute.rs:215:25 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(6244)): execute: too many cycle iterations`
_directives_deprecated_library.py:15:31: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `int`
_directives_deprecated_library.py:30:26: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `str`
_directives_deprecated_library.py:36:41: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `Self@__add__` |
|
7bccf03 to
6b1cb8f
Compare
|
I think you are technically correct here, but I'm a bit worried about being strict about this, because apparently both mypy and pyright allow it. (By which I mean, using |
|
Well, here are some options I can think of:
I could try (4)? It might be interesting. It does feel like something of an elaborate hack in the name of compatibility, but maybe that's worth it. FWIW, the long-term plan at typeshed is to replace the current |
|
Yes, I agree that (1) is not a good option. I wonder what other type checkers are doing here? I suspect it's not (4) -- it's probably some version of (2)? I feel like there's another possibility here, where we get effectively the same semantics as (2) or (4), but just via a new Type variant instead (created by use of (I think the additional semantics of (4) where you could make your own subtype of Ultimately I think we do need to support this in some form. That is, I don't think (3) alone is a workable option either, although I do think some of this PR could still make sense, just so we understand the value-type of I'm pretty sure I recall that mypy used to not support |
Hmm, really? That happened before I "entered the scene", if so 😄 |
|
It happened in late 2021, in python/mypy#11162, but it doesn't look like there was a lot of user feedback there, just one user report that Nikita agreed with, and convinced Jelle and Jukka to merge it :) I do feel like I remember an earlier issue about it with more discussion, but not finding that at the moment. |
This is arguable, I guess -- if the main purpose of such an annotation is that it lets you write code that generically introspects NamedTuples, then a Protocol (that can also be satisfied by a manually constructed class) seems like a pretty sensible implementation? |
|
Looks like some previous discussion was at python/typing#431 prior to that mypy PR |
|
Here's the section in the mypy docs describing this feature...
|
|
Found the thread I was thinking of: python/mypy#3915 Complete with comments from both you and I 😆 |
|
Here's my best attempt at a protocol that all import sys
from typing import Reversible, Iterable, Collection, SupportsIndex, Protocol, overload, ClassVar, Any, Self
class NamedTuplesque(Reversible[object], Collection[object], Protocol):
# from typing.NamedTuple stub
_field_defaults: ClassVar[dict[str, Any]]
_fields: ClassVar[tuple[str, ...]]
@classmethod
def _make(self: Self, iterable: Iterable[Any]) -> Self: ...
def _asdict(self, /) -> dict[str, Any]: ...
def _replace(self: Self, /, **kwargs) -> Self: ...
if sys.version_info >= (3, 13):
def __replace__(self: Self, **kwargs) -> Self: ...
# from Sequence stub
@overload
def __getitem__(self, index: int, /) -> object: ...
@overload
def __getitem__(self, index: slice, /) -> tuple[object, ...]: ...
def index(self, value, start: int = 0, stop: int = ..., /) -> int: ...
def count(self, value, /) -> int: ...
# from tuple stub
def __add__(self, value: tuple[Any, ...], /) -> tuple[object, ...]: ...
def __mul__(self, value: SupportsIndex, /) -> tuple[object, ...]: ...
def __rmul__(self, value: SupportsIndex, /) -> tuple[object, ...]: ...
def __hash__(self, /) -> int: ...We seem to do pretty well at recognising that But the stub there is complicated enough that it honestly might be less of a maintenance burden to just add a new |
|
I guess there's some redundancy with It seems like if we use a dedicated Seems like you did the hardest part already by writing the protocol; sticking it in Nit: we could name it |
Yes -- possibly |
I still feel like the "right" thing to do is to ban it in type expressions entirely, especially since the mypy docs explicitly call out that their feature is
But the discussions you've linked to are pretty persuasive that users "expect" it to work in type expressions like a protocol type, and compatibility with other type checkers is obviously valuable. So I'll make the change 🙃 |
caafbc7 to
a587de5
Compare
|
The primer report is now much less dramatic for the revised version of the PR. We now just have two diagnostics going away in static-frame because we now have a better understanding of the code patterns they're using that are supposed to work with any The initial attempt to use a protocol type resulted in one new diagnostic being added in django-stubs, because we did not see
I've since removed that diagnostic by making the protocol in |
For the record I'm still not convinced this was a good idea. python/mypy#11162 (comment) But it seems the approach you came up with in this PR is reasonable. |
If you'd pushed back harder back then, we wouldn't be in this situation now 😆 Seriously though, this feature doesn't bother me much. I feel like "having a protocol for NamedTuple-created classes" makes sense and has good use cases, and having that protocol maintained in a shared place rather than by every individual user also makes sense to me. So really the only "questionable" thing here is having the special form |
carljm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thank you!!
* main: [ty] Represent `NamedTuple` as an opaque special form, not a class (#19915) [ty] Remove incorrect type narrowing for `if type(x) is C[int]` (#19926) Bump Rust MSRV to 1.87 (#19924) Add `else`-branch narrowing for `if type(a) is A` when `A` is `@final` (#19925) [ty] Sync vendored typeshed stubs (#19923) [ty] fix lazy snapshot sweeping in nested scopes (#19908)
Summary
At runtime,
NamedTupleis a function:__kwdefaults____mro__NamedTuple"NamedTuple" is actually just syntactic sugar for creating a tuple subclass that has a number of additional properties and methods monkey-patched onto it.Ty understands the last of these points: given the class definition
Pointhere, we accurately infer thatPointdirectly inherits fromtuple[int, int]and does not haveNamedTuplein its MRO:However, it currently believes that
NamedTupleis a class at runtime, due to the definition that typeshed gives intyping.pyi:ruff/crates/ty_vendored/vendor/typeshed/stdlib/typing.pyi
Lines 1734 to 1775 in dc2e8ab
I've argued elsewhere that the current typeshed definition for
NamedTuplemakes little sense and that it would be simpler for typeshed to describeNamedTupleas what it actually is at runtime: a function! Unfortunately, however, it's hard to change the definition in typeshed at this point, as other type checkers rely on it being this way.This PR therefore adds some special casing to our type inference logic so that if we see that a class definition is a class with the name
"NamedTuple"and it comes from the moduletypingor the moduletyping_extensions, we override our usual type inference logic and inferType::SpecialForm(SpecialFormType::NamedTuple)for the class instead of aType::ClassLiteral()type. This allows us to add a bespoke error message if users try to use the function in a type expression (currently this does not work -- because we accurately do not infer any class as being a "subclass ofNamedTuple" -- but we also do not emit a diagnostic for it, causing confusion). Attribute access on the newSpecialFormTypevariant falls back to attribute access on instances oftypes.FunctionType, meanwhile, staying true to the actual semantics thattyping.NamedTuplehas at runtime.Test Plan
Mdtests.