Skip to content

Conversation

@ibraheemdev
Copy link
Member

Summary

Resolves astral-sh/ty#1408.

@ibraheemdev ibraheemdev requested a review from carljm as a code owner October 24, 2025 01:35
@ibraheemdev ibraheemdev added the ty Multi-file analysis & type inference label Oct 24, 2025
@ibraheemdev ibraheemdev force-pushed the ibraheem/bidi-constructors branch from 06c5bd7 to 445a1dc Compare October 24, 2025 01:37
@github-actions
Copy link
Contributor

github-actions bot commented Oct 24, 2025

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@github-actions
Copy link
Contributor

github-actions bot commented Oct 24, 2025

mypy_primer results

No ecosystem changes detected ✅

Memory usage changes were detected when running on open source projects
prefect (https://github.com/PrefectHQ/prefect)
-     struct fields = ~36MB
+     struct fields = ~38MB
-     memo metadata = ~108MB
+     memo metadata = ~113MB

@github-actions
Copy link
Contributor

ecosystem-analyzer results

No diagnostic changes detected ✅
Full report with detailed diff (timing results)

@ibraheemdev
Copy link
Member Author

ibraheemdev commented Oct 24, 2025

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).

Comment on lines 209 to 213
class T1(TypedDict):
x: int

class T2(TypedDict):
x: int
Copy link
Member

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?

Copy link
Member Author

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"
Copy link
Member

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)

Copy link
Member Author

@ibraheemdev ibraheemdev Oct 24, 2025

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 😄

@AlexWaygood AlexWaygood removed their request for review October 24, 2025 13:37
@ibraheemdev ibraheemdev enabled auto-merge (squash) October 24, 2025 20:13
@ibraheemdev ibraheemdev merged commit 304ac22 into main Oct 24, 2025
40 checks passed
@ibraheemdev ibraheemdev deleted the ibraheem/bidi-constructors branch October 24, 2025 20:14
dcreager added a commit that referenced this pull request Oct 27, 2025
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ecosystem-analyzer ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bidirectional inference does not consider parameter types of constructors

3 participants