-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] typecheck dict methods for TypedDict
#19874
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
771f1d1 to
3902667
Compare
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-08-29 13:40:04.333301009 +0000
+++ new-output.txt 2025-08-29 13:40:06.963314808 +0000
@@ -849,7 +849,6 @@
tuples_type_form.py:25:1: error[invalid-assignment] Object of type `tuple[Literal[1]]` is not assignable to `tuple[()]`
tuples_type_form.py:36:1: error[invalid-assignment] Object of type `tuple[Literal[1], Literal[2], Literal[3], Literal[""]]` is not assignable to `tuple[int, ...]`
typeddicts_operations.py:37:5: error[missing-typed-dict-key] Missing required key 'name' in TypedDict `Movie` constructor
-typeddicts_operations.py:60:1: error[type-assertion-failure] Argument does not have asserted type `str | None`
typeddicts_operations.py:62:1: error[unresolved-attribute] Type `MovieOptional` has no attribute `clear`
typeddicts_readonly_inheritance.py:65:1: error[missing-typed-dict-key] Missing required key 'name' in TypedDict `RequiredName` constructor
typeddicts_type_consistency.py:69:21: error[invalid-key] Invalid key access on TypedDict `A3`: Unknown key "y"
@@ -859,5 +858,5 @@
typeddicts_usage.py:28:1: error[missing-typed-dict-key] Missing required key 'name' in TypedDict `Movie` constructor
typeddicts_usage.py:28:18: error[invalid-key] Invalid key access on TypedDict `Movie`: Unknown key "title"
typeddicts_usage.py:40:24: error[invalid-type-form] The special form `typing.TypedDict` is not allowed in type expressions. Did you mean to use a concrete TypedDict or `collections.abc.Mapping[str, object]` instead?
-Found 860 diagnostics
+Found 859 diagnostics
WARN A fatal error occurred while checking some files. Not all project files were analyzed. See the diagnostics list above for details. |
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-return-type |
4 | 0 | 0 |
invalid-argument-type |
3 | 0 | 0 |
possibly-unbound-attribute |
2 | 0 | 0 |
unused-ignore-comment |
0 | 2 | 0 |
invalid-key |
1 | 0 | 0 |
redundant-cast |
1 | 0 | 0 |
| Total | 11 | 2 | 0 |
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.
Thank you very much. This looks great.
I looked through a few ecosystem changes here (and found a problem with .get calls with a default argument of a different type, which I commented on inline), but it might be worth going through a few more to see if we're missing anything in the implementation.
|
I'm currently looking into making the required changes here. |
4626add to
476cc3e
Compare
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.
Just a couple of comments re the generics/typevar changes.
| )), | ||
| ) | ||
| })) | ||
| .collect(); |
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.
nit: CallableSignature::from_overloads takes in an iterator, so I think you can skip the collect here
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.
Oops, yes. I missed this in my review. Thanks.
|
Ok, after adding some fallback overloads for discord.py (https://github.com/Rapptz/discord.py)
- discord/activity.py:882:38: warning[unused-ignore-comment] Unused blanket `type: ignore` directive
- discord/member.py:390:41: warning[unused-ignore-comment] Unused blanket `type: ignore` directive
- Found 533 diagnostics
+ Found 531 diagnosticsCorrect, they are openlibrary (https://github.com/internetarchive/openlibrary)
+ openlibrary/plugins/worksearch/code.py:447:34: error[invalid-argument-type] Argument to function `__new__` is incorrect: Expected `Iterable[Unknown]`, found `list[str] | None`
+ openlibrary/plugins/worksearch/code.py:447:61: error[invalid-argument-type] Argument to function `__new__` is incorrect: Expected `Iterable[Unknown]`, found `list[str] | None`
- Found 709 diagnostics
+ Found 711 diagnosticsTrue positives. They have a key with value type meson (https://github.com/mesonbuild/meson)
+ mesonbuild/interpreter/interpreter.py:1804:69: error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `dict[str, str] | None`, found `dict[OptionKey, str | int | list[str]]`
+ mesonbuild/modules/python.py:169:42: error[invalid-key] Invalid key access on TypedDict `ExtensionModuleKw`: Unknown key "limited_api"
- Found 808 diagnostics
+ Found 810 diagnosticsBoth look like true positives to me prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/runner/storage.py:851:13: error[invalid-return-type] Return type does not match returned value: expected `str`, found `str | (Any & ~AlwaysFalsy & ~Secret[Unknown]) | (Secret[str] & ~AlwaysFalsy & ~Secret[Unknown]) | (@Todo(Type::Intersection.call()) & ~AlwaysFalsy)`
+ src/prefect/runner/storage.py:858:13: error[invalid-return-type] Return type does not match returned value: expected `str`, found `(Any & ~AlwaysFalsy & ~Secret[Unknown]) | str | (Secret[str] & ~AlwaysFalsy & ~Secret[Unknown]) | (@Todo(Type::Intersection.call()) & ~AlwaysFalsy)`
+ src/prefect/runner/storage.py:859:16: warning[possibly-unbound-attribute] Attribute `startswith` on type `(Any & ~AlwaysFalsy & ~Secret[Unknown]) | (str & ~AlwaysFalsy) | (Secret[str] & ~AlwaysFalsy & ~Secret[Unknown]) | (@Todo(Type::Intersection.call()) & ~AlwaysFalsy)` is possibly unbound
+ src/prefect/runner/storage.py:866:13: error[invalid-return-type] Return type does not match returned value: expected `str`, found `str | (Any & ~AlwaysFalsy & ~Secret[Unknown]) | (Secret[str] & ~AlwaysFalsy & ~Secret[Unknown]) | (@Todo(Type::Intersection.call()) & ~AlwaysFalsy)`
+ src/prefect/runner/storage.py:867:20: warning[possibly-unbound-attribute] Attribute `startswith` on type `(Any & ~AlwaysFalsy & ~Secret[Unknown]) | (str & ~AlwaysFalsy) | (Secret[str] & ~AlwaysFalsy & ~Secret[Unknown]) | (@Todo(Type::Intersection.call()) & ~AlwaysFalsy)` is possibly unbound
+ src/prefect/runner/storage.py:872:12: error[invalid-return-type] Return type does not match returned value: expected `str`, found `(Any & ~AlwaysFalsy & ~Secret[Unknown]) | (str & ~AlwaysFalsy) | (Secret[str] & ~AlwaysFalsy & ~Secret[Unknown]) | (@Todo(Type::Intersection.call()) & ~AlwaysFalsy)`
- Found 3013 diagnostics
+ Found 3019 diagnosticsSeems unrelated to dd-trace-py (https://github.com/DataDog/dd-trace-py)
+ ddtrace/llmobs/_experiment.py:506:28: warning[redundant-cast] Value is already of type `int`
- Found 6602 diagnostics
+ Found 6603 diagnosticsTrue positive |
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.
Thanks again!
| # error: [invalid-argument-type] "Cannot pop required field 'name' from TypedDict `Person`" | ||
| reveal_type(p.pop("name")) # revealed: Unknown |
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.
I feel like ideally we'd still infer str here (despite the fact that the operation is illegal, it seems pretty clear that the result of this operation will always be a str), but I can see that that might be difficult to do consistently with the synthesized-overload approach. And consistently inferring Unknown here seems better than sometimes inferring Unknown, sometimes str (depending on exactly how the method is invoked on the typeddict object).
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.
I feel like ideally we'd still infer
strhere (despite the fact that the operation is illegal, it seems pretty clear that the result of this operation will always be astr)
Yes, interesting point. I agree that this would probably be the better return type, but as you said, it's not possible to achieve that with synthesized overloads, unless we manage to synthesize overloads that trigger specific CallErrors when selected, or similar. Until then, Unknown is not ideal, but it's also not wrong. And if you're going to # type: ignore your illegal .pop operation, you might as well throw in a cast or a redeclaration :-)
* main: (28 commits) [ty] `__class_getitem__` is a classmethod (#20192) [ty] Support `__init_subclass__` (#20190) Update dependency ruff to v0.12.11 (#20184) Update rui314/setup-mold digest to 725a879 (#20181) Update CodSpeedHQ/action action to v3.8.1 (#20183) Update cargo-bins/cargo-binstall action to v1.15.3 (#20182) Update Rust crate camino to v1.1.12 (#20185) Update Rust crate clap to v4.5.46 (#20186) Update Rust crate mimalloc to v0.1.48 (#20187) [ty] Sync vendored typeshed stubs (#20188) [ty] improve cycle-detection coverage for apply_type_mapping (#20159) [ty] don't assume that deferred type inference means deferred name resolution (#20160) Less confidently mark f-strings as empty when inferring truthiness (#20152) [ty] skip a slow seed in fuzzer (#20161) Revert "[ty] Use `invalid-assignment` error code for invalid assignments to `ClassVar`s" (#20158) [ty] add six ecosystem projects to good.txt (#20157) [ty] Use `invalid-assignment` error code for invalid assignments to `ClassVar`s (#20156) [ty] minor TypedDict fixes (#20146) [ty] ensure union normalization really normalizes (#20147) [ty] typecheck dict methods for `TypedDict` (#19874) ...
## Summary
Typecheck `get()`, `setdefault()`, `pop()` for `TypedDict`
```py
from typing import TypedDict
from typing_extensions import NotRequired
class Employee(TypedDict):
name: str
department: NotRequired[str]
emp = Employee(name="Alice", department="Engineering")
emp.get("name")
emp.get("departmen", "Unknown")
emp.pop("department")
emp.pop("name")
```
<img width="838" height="529" alt="Screenshot 2025-08-12 at 11 42 12"
src="https://github.com/user-attachments/assets/77ce150a-223c-4931-b914-551095d8a3a6"
/>
part of astral-sh/ty#154
## Test Plan
Updated Markdown tests
---------
Co-authored-by: David Peter <[email protected]>
Summary
Typecheck
get(),setdefault(),pop()forTypedDictpart of astral-sh/ty#154
Test Plan
Updated Markdown tests