[ty] prefer submodule over module __getattr__ in from-imports #21260
Conversation
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
Contributor
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
Contributor
|
Contributor
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
unresolved-attribute |
85 | 347 | 0 |
invalid-argument-type |
16 | 1 | 0 |
invalid-parameter-default |
7 | 0 | 0 |
possibly-unresolved-reference |
0 | 6 | 0 |
unused-ignore-comment |
0 | 6 | 0 |
invalid-return-type |
1 | 3 | 0 |
invalid-assignment |
1 | 0 | 2 |
possibly-missing-attribute |
2 | 0 | 1 |
unsupported-operator |
0 | 0 | 3 |
too-many-positional-arguments |
2 | 0 | 0 |
redundant-cast |
0 | 1 | 0 |
| Total | 114 | 364 | 6 |
carljm
requested review from
AlexWaygood,
dcreager and
sharkdp
as code owners
AlexWaygood
approved these changes
Nov 3, 2025
crates/ty_python_semantic/resources/mdtest/import/module_getattr.md
Outdated
Show resolved
Hide resolved
…tr.md Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes astral-sh/ty#1053
Summary
Other type checkers prioritize a submodule over a package
__getattr__infrom mod import sub, even though the runtime precedence is the other direction. In effect, this is making an implicit assumption that a module__getattr__will not handle (that is, will raiseAttributeError) for names that are also actual submodules, rather than shadowing them. In practice this seems like a realistic assumption in the ecosystem? Or at least the ecosystem has adapted to it, and we need to adapt this precedence also, for ecosystem compatibility.The implementation is a bit ugly, precisely because it departs from the runtime semantics, and our implementation is oriented toward modeling runtime semantics accurately. That is,
__getattr__is modeled within the member-lookup code, so it's hard to split "member lookup result from module__getattr__" apart from other member lookup results. I did this via a syntheticTypeQualifier::FROM_MODULE_GETATTRthat we attach to a type resulting from a member lookup, which isn't beautiful but it works well and doesn't introduce inefficiency (e.g. redundant member lookups).Test Plan
Updated mdtests.
Also added a related mdtest formalizing our support for a module
__getattr__that is explicitly annotated to accept a limited set of names. In principle this could be an alternative (more explicit) way to handle the precedence problem without departing from runtime semantics, if the ecosystem would adopt it.Ecosystem analysis
Lots of removed diagnostics which are an improvement because we now infer the expected submodule.
Added diagnostics are mostly unrelated issues surfaced now because we previously had an earlier attribute error resulting in
Unknown; now we correctly resolve the module so that earlier attribute error goes away, we get an actual type instead ofUnknown, and that triggers a new error.In scipy and sklearn, the module
__getattr__which we were respecting previously is un-annotated so returned a forgivingUnknown; now we correctly see the actual module, which reveals some cases of astral-sh/ty#133 that were previously hidden (scipy/optimize/__init__.pyimportsfrom ._tnc.)