Skip to content

[ty] refactor Place #20871

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 15, 2025
Merged

[ty] refactor Place #20871

merged 3 commits into from
Oct 15, 2025

Conversation

@mtshiba
Copy link
Contributor

@mtshiba mtshiba commented Oct 14, 2025 •
edited
Loading

Summary

Part of astral-sh/ty#1341

The following changes will be made to Place.

  • Introduce TypeOrigin
  • Place::Type -> Place::Defined
  • Place::Unbound -> Place::Undefined
  • Boundness -> Definedness

TypeOrigin::Declared+Definedness::PossiblyUndefined are patterns that weren't considered before, but this PR doesn't address them yet, only refactors.

Test Plan

N/A

@mtshiba
[ty] refactor Place
e432554 
* Introduce `TypeOrigin`
* `Place::Type` -> `Place::Defined`
* `Place::Unbound` -> `Place::Undefined`
* `Boundness` -> `Definedness`
@github-actions
Copy link
Contributor

github-actions bot commented Oct 14, 2025 •
edited
Loading

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 14, 2025 •
edited
Loading

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

@mtshiba
Copy link
Contributor Author

mtshiba commented Oct 14, 2025

Hmm, the diff in mypy_primer is unexpected.
There was no diff in the initial commit, and it appears that the diff was caused by a merge commit that should have been unrelated.

It seems that this also occurred in #20792 and #20806.
I don't know the cause, but perhaps there is non-deterministic inference being made regarding dataclasses.field or overloading?

@mtshiba mtshiba marked this pull request as ready for review October 14, 2025 19:54
@amyreese amyreese added the ty Multi-file analysis & type inference label Oct 14, 2025
@AlexWaygood
Copy link
Member

but perhaps there is non-deterministic inference being made regarding dataclasses.field or overloading?

Yeah, there does seem to be something nondeterministic happening there. It's quite annoying; if you feel like tracking it down, that would be very helpful 😄

sharkdp
sharkdp approved these changes Oct 15, 2025
View reviewed changes
Copy link
Contributor

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing this! Just one comment.

If someone is unhappy with the newly proposed names here, we can always easily change them later. I think we should aim to get this refactor in as quick as possible, or it will collect a lot of conflicts.

dcreager
dcreager approved these changes Oct 15, 2025
View reviewed changes
Copy link
Member

@dcreager dcreager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for tackling this! I agree with David that we should get this in sooner rather than later, and tackle any proposed changes as follow ons. This looks good enough to land to me!

@AlexWaygood AlexWaygood removed their request for review October 15, 2025 12:55
@mtshiba mtshiba requested a review from sharkdp October 15, 2025 17:32
@sharkdp sharkdp merged commit 9de34e7 into astral-sh:main Oct 15, 2025
41 checks passed
@sharkdp
Copy link
Contributor

sharkdp commented Oct 15, 2025

Thank you!

@mtshiba mtshiba deleted the refactor-place branch October 16, 2025 00:14
dcreager added a commit that referenced this pull request Oct 16, 2025
@dcreager
Merge remote-tracking branch 'origin/main' into dcreager/non-non-infe…
642ed43 
…rable

* origin/main:
  Don't use codspeed or depot runners in CI jobs on forks (#20894)
  [ty] cache Type::is_redundant_with (#20477)
  Fix run-away for mutually referential instance attributes (#20645)
  [ty] Limit shown import paths to at most 5 unless ty runs with `-v` (#20912)
  [ty] Use field-specifier return type as the default type for the field (#20915)
  [ty] Do not assume that `field`s have a default value (#20914)
  [ty] Fix match pattern value narrowing to use equality semantics (#20882)
  Update setup instructions for Zed 0.208.0+ (#20902)
  Move TOML indent size config (#20905)
  [syntax-errors]: implement F702 as semantic syntax error (#20869)
  [ty] Heterogeneous unpacking support for unions (#20377)
  [ty] refactor `Place` (#20871)
  Auto-accept snapshot changes as part of typeshed-sync PRs (#20892)
  [`airflow`] Add warning to `airflow.datasets.DatasetEvent` usage (`AIR301`) (#20551)
  [`flake8-pyi`] Fix operator precedence by adding parentheses when needed (`PYI061`) (#20508)
  [`pyupgrade`] Fix false negative for `TypeVar` with default argument in `non-pep695-generic-class` (`UP046`) (#20660)
  Update parser snapshots (#20893)
  Fix syntax error false positives for escapes and quotes in f-strings (#20867)
dcreager added a commit that referenced this pull request Oct 16, 2025
@dcreager
Merge branch 'dcreager/non-non-inferable' into dcreager/real-constrai…
bb6cf4d 
…nt-sets

* dcreager/non-non-inferable:
  Don't use codspeed or depot runners in CI jobs on forks (#20894)
  [ty] cache Type::is_redundant_with (#20477)
  Fix run-away for mutually referential instance attributes (#20645)
  [ty] Limit shown import paths to at most 5 unless ty runs with `-v` (#20912)
  [ty] Use field-specifier return type as the default type for the field (#20915)
  [ty] Do not assume that `field`s have a default value (#20914)
  [ty] Fix match pattern value narrowing to use equality semantics (#20882)
  Update setup instructions for Zed 0.208.0+ (#20902)
  Move TOML indent size config (#20905)
  [syntax-errors]: implement F702 as semantic syntax error (#20869)
  [ty] Heterogeneous unpacking support for unions (#20377)
  [ty] refactor `Place` (#20871)
  Auto-accept snapshot changes as part of typeshed-sync PRs (#20892)
  [`airflow`] Add warning to `airflow.datasets.DatasetEvent` usage (`AIR301`) (#20551)
  [`flake8-pyi`] Fix operator precedence by adding parentheses when needed (`PYI061`) (#20508)
  [`pyupgrade`] Fix false negative for `TypeVar` with default argument in `non-pep695-generic-class` (`UP046`) (#20660)
  Update parser snapshots (#20893)
  Fix syntax error false positives for escapes and quotes in f-strings (#20867)
@sharkdp sharkdp mentioned this pull request Oct 16, 2025
Merged
@mtshiba mtshiba mentioned this pull request Oct 17, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dcreager dcreager dcreager approved these changes

@carljm carljm Awaiting requested review from carljm carljm is a code owner

@sharkdp sharkdp Awaiting requested review from sharkdp sharkdp is a code owner

Assignees

No one assigned

Labels

ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mtshiba @AlexWaygood @sharkdp @dcreager @amyreese