-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Use constructor parameter types as type context #21054
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
06c5bd7 to
445a1dc
Compare
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
445a1dc to
5663132
Compare
|
|
The mypy-primer diff changes are due to a known issue with duplicated diagnostics (see astral-sh/ty#1428), there are no actual new diagnostics (somewhat surprisingly). |
| class T1(TypedDict): | ||
| x: int | ||
|
|
||
| class T2(TypedDict): | ||
| x: int |
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.
Is it important that these are separate TypedDict types? If so, can you call out why in a comment or in the markdown documentation?
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 realized this was actually incorrect because of structural subtyping. The idea was that both type contexts would be required for a valid inference, I updated the test to use a generic call instead.
| ```py | ||
| from typing import Literal | ||
|
|
||
| a: Literal["x"] = "x" |
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.
The examples in this section look like they're not exercising bidi typing to their fullest, since the expressions will already be inferred as Literal["x"] even without the type context. Would it be possible to update these so that the type context is different from what we'd normally infer for the expression? (Maybe a list literal and a more specific list[something] type context?) That might overlap with some of the tests elsewhere in the file, but I think it would make this section clearer.
(And ideally, it would be nice to be able to then show that the bidi-inferred type is different, by placing this alongside the same expression without a type context. But I think that would require hover annotations #20786)
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.
Sorry, that was an oversight, fixed. hover tests would nice 😄
* origin/main: Respect `--output-format` with `--watch` (#21097) [`pyflakes`] Revert to stable behavior if imports for module lie in alternate branches for `F401` (#20878) Fix finding keyword range for clause header after statement ending with semicolon (#21067) [ty] Fix bug where ty would think all types had an `__mro__` attribute (#20995) Restore `indent.py` (#21094) [`flake8-django`] Apply `DJ001` to annotated fields (#20907) Clearer error message when `line-length` goes beyond threshold (#21072) Update upload and download artifacts github actions (#21083) Update dependency mdformat-mkdocs to v4.4.2 (#21088) Update cargo-bins/cargo-binstall action to v1.15.9 (#21086) Update Rust crate clap to v4.5.50 (#21090) Update Rust crate get-size2 to v0.7.1 (#21091) Update Rust crate bstr to v1.12.1 (#21089) Add missing docstring sections to the numpy list (#20931) [`pydoclint`] Fix false positive on explicit exception re-raising (`DOC501`, `DOC502`) (#21011) [ty] Use constructor parameter types as type context (#21054)
Summary
Resolves astral-sh/ty#1408.