[ty] basic narrowing on attribute and subscript expressions#17643
Merged
sharkdp merged 21 commits intoastral-sh:mainfrom Jun 17, 2025
Merged
[ty] basic narrowing on attribute and subscript expressions#17643sharkdp merged 21 commits intoastral-sh:mainfrom
sharkdp merged 21 commits intoastral-sh:mainfrom
Conversation
mtshiba
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
Contributor
|
mtshiba
force-pushed
the
attr-subscript-narrowing
branch
from
a9ee6ed to
cfa79a2
Compare
Contributor
|
Thank you for working on this!
I think it's not just a matter of additional narrowing. It looks to me like, on this branch, we can infer incorrect types by not considering attribute assignments: from typing import reveal_type
class C:
x: int | None = None
c = C()
if c.x is None:
c.x = 1
reveal_type(c.x) # this branch: NoneI'm afraid this is something that we need to fix before we can consider merging this. It also leads to some new false positives in the ecosystem checks. Please let me know if you need any help with this. |
mtshiba
force-pushed
the
attr-subscript-narrowing
branch
from
cfa79a2 to
9db9728
Compare
3 tasks
This comment was marked as outdated.
This comment was marked as outdated.
carljm
added a commit
that referenced
this pull request
Jun 5, 2025
## Summary This PR partially solves astral-sh/ty#164 (derived from #17643). Currently, the definitions we manage are limited to those for simple name (symbol) targets, but we expand this to track definitions for attribute and subscript targets as well. This was originally planned as part of the work in #17643, but the changes are significant, so I made it a separate PR. After merging this PR, I will reflect this changes in #17643. There is still some incomplete work remaining, but the basic features have been implemented, so I am publishing it as a draft PR. Here is the TODO list (there may be more to come): * [x] Complete rewrite and refactoring of documentation (removing `Symbol` and replacing it with `Place`) * [x] More thorough testing * [x] Consolidation of duplicated code (maybe we can consolidate the handling related to name, attribute, and subscript) This PR replaces the current `Symbol` API with the `Place` API, which is a concept that includes attributes and subscripts (the term is borrowed from Rust). ## Test Plan `mdtest/narrow/assignment.md` is added. --------- Co-authored-by: David Peter <[email protected]> Co-authored-by: Carl Meyer <[email protected]>
mtshiba
force-pushed
the
attr-subscript-narrowing
branch
from
7417ba6 to
0c8996c
Compare
Collaborator
Author
|
The Upon investigation, it appears that this is in fact a known bug. I minimized the repro code and got: while True:
...
if not size:
break
try:
...
self._total_write_done_index += num_bytes
except BlockingIOError:
break
...
if index > self._total_write_done_index:
...I think this is another example of astral-sh/ty#365. |
sharkdp
changed the title
sharkdp
requested changes
Jun 10, 2025
Contributor
sharkdp
left a comment
sharkdp
left a comment
There was a problem hiding this comment.
Thank you very much — this looks great.
Unfortunately, I think I found some cases that are not yet handled correctly when we combine assignments to attributes/objects with narrowing.
crates/ty_python_semantic/resources/mdtest/narrow/complex_target.md
Outdated
Show resolved
Hide resolved
crates/ty_python_semantic/resources/mdtest/narrow/complex_target.md
Outdated
Show resolved
Hide resolved
This was referenced Jun 10, 2025
…et.md Co-authored-by: David Peter <[email protected]>
sharkdp
reviewed
Jun 16, 2025
sharkdp
reviewed
Jun 16, 2025
sharkdp
reviewed
Jun 16, 2025
sharkdp
reviewed
Jun 16, 2025
…n the assignments
Contributor
|
I can take care of fixing the merge conflicts here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR closes astral-sh/ty#164.
This PR introduces a basic type narrowing mechanism for attribute/subscript expressions.
Member accesses, int literal subscripts, string literal subscripts are supported (same as mypy and pyright).
Advanced narrowing mechanisms, such as the following, are not implemented in this PR (and will be future works):
narrowing by overwriting assignments[ty] type narrowing by attribute/subscript assignments #18041Test Plan
New test cases are added to
mdtest/narrow/complex_target.md.