[ty] introduce local variables for from imports of submodules in __init__.py(i)
#21173
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 testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-11-10 23:02:35.450360648 +0000
+++ new-output.txt 2025-11-10 23:02:38.823377146 +0000
@@ -1,4 +1,4 @@
-fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/05a9af7/src/function/execute.rs:451:17 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(19371)): execute: too many cycle iterations`
+fatal[panic] Panicked at /home/runner/.cargo/git/checkouts/salsa-e6f3bb7c2a062968/05a9af7/src/function/execute.rs:451:17 when checking `/home/runner/work/ruff/ruff/typing/conformance/tests/aliases_typealiastype.py`: `infer_definition_types(Id(19384)): execute: too many cycle iterations`
_directives_deprecated_library.py:15:31: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `int`
_directives_deprecated_library.py:30:26: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `str`
_directives_deprecated_library.py:36:41: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `Self@__add__`
|
Contributor
|
Contributor
|
Contributor
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
unresolved-attribute |
6 | 612 | 10 |
invalid-assignment |
7 | 0 | 0 |
invalid-argument-type |
2 | 1 | 0 |
unused-ignore-comment |
3 | 0 | 0 |
unresolved-import |
0 | 2 | 0 |
call-non-callable |
1 | 0 | 0 |
possibly-missing-import |
1 | 0 | 0 |
possibly-unresolved-reference |
0 | 1 | 0 |
| Total | 20 | 616 | 10 |
Contributor
Author
|
Preserving for future comparison: this is the "submodule-attributes" approach's results. I am now pushing up a different WIP implementation. 
|
CodSpeed Performance ReportMerging #21173 will not alter performanceComparing Summary
|
Contributor
Just FYI, you can also access the full history here, I do that quite often after pushing a change. 
|
Contributor
Author
|
I have cleaned up the implementation and added some more tests. I've made a few edge-cases a bit more strict. I am hoping this is a complete implementation, except for some code/mdtest comments need to be updated to reflect the current semantics. For posterity, this is the results that we agreed seemed shippable before my cleanup. Hopefully the stricter implementation isn't a huge change! 
|
Contributor
Author
|
I believe the TL;DR of this PR is now:
As a reminder, "re-exported" is only an applicable concern for definitions in .pyi files, so both kinds of definitions will be fully visible in .py files. |
Gankra
requested review from
AlexWaygood,
MichaReiser,
carljm,
dcreager and
sharkdp
as code owners
Gankra
changed the title
from..import effectsfrom imports of submodules in __init__.py(i)
This was referenced Nov 5, 2025
available_submodule_attributes should probably be considered last instead of first
astral-sh/ty#1488
Open
Contributor
Author
|
See astral-sh/ty#133 (comment) for a list of followups that I am intentionally punting on in this PR (or the sea of references right above this comment...) |
carljm
approved these changes
Nov 10, 2025
Contributor
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
This is great, thank you!
Normally I would want to adjust more of the wordings and tests in this PR to accurately describe the behavior I think we should be aiming for (with TODOs as needed where we don't achieve it yet), but I'm good with skipping those updates here if the follow-up PRs are coming shortly. So consider all comments on this PR as OK to address in follow-ups if you prefer.
Comment on lines
3
to
+5
| This document covers ty-specific extensions to the | ||
| [standard import conventions](https://typing.python.org/en/latest/spec/distributing.html#import-conventions). | ||
|
|
||
| It's a common idiom for a package's `__init__.py(i)` to include several imports like | ||
| `from . import mysubmodule`, with the intent that the `mypackage.mysubmodule` attribute should work | ||
| for anyone who only imports `mypackage`. | ||
|
|
||
| In the context of a `.py` we handle this well through our general attempts to faithfully implement | ||
| import side-effects. However for `.pyi` files we are expected to apply | ||
| [a more strict set of rules](https://typing.python.org/en/latest/spec/distributing.html#import-conventions) | ||
| to encourage intentional API design. Although `.pyi` files are explicitly designed to work with | ||
| typecheckers, which ostensibly should all enforce these strict rules, every typechecker has its own | ||
| defacto "extensions" to them and so a few idioms like `from . import mysubmodule` have found their | ||
| way into `.pyi` files too. | ||
|
|
||
| Thus for the sake of compatibility, we need to define our own "extensions". Any extensions we define | ||
| here have several competing concerns: | ||
|
|
||
| - Extensions should ideally be kept narrow to continue to encourage explicit API design | ||
| - Extensions should be easy to explain, document, and understand | ||
| - Extensions should ideally still be a subset of runtime behaviour (if it works in a stub, it works | ||
| at runtime) | ||
| - Extensions should ideally not make `.pyi` files more permissive than `.py` files (if it works in a | ||
| stub, it works in an impl) | ||
|
|
||
| To that end we define the following extension: | ||
|
|
||
| > If an `__init__.pyi` for `mypackage` contains a `from...import` targetting a direct submodule of | ||
| > `mypackage`, then that submodule should be available as an attribute of `mypackage`. | ||
| [standard import conventions](https://typing.python.org/en/latest/spec/distributing.html#import-conventions), | ||
| and other intentional deviations from actual python semantics. |
Contributor
There was a problem hiding this comment.
The current text below buries (or eliminates) the lede: we dive straight into the edgiest of edge cases (like submodule imports inside a function) without any discussion of the general/common cases and how we choose to model these "submodule attributes" in general.
I think it probably makes most sense to do an update here separately, after a few more PRs in this area land. I'm happy to do the prose update myself since I have opinions about how we present it.
Comment on lines
+9
to
+15
| - **froms are locals**: a `from..import` can only define locals, it does not have global | ||
| side-effects. Specifically any submodule attribute `a` that's implicitly introduced by either | ||
| `from .a import b` or `from . import a as b` (in an `__init__.py(i)`) is a local and not a | ||
| global. If you do such an import at the top of a file you won't notice this. However if you do | ||
| such an import in a function, that means it will only be function-scoped (so you'll need to do | ||
| it in every function that wants to access it, making your code less sensitive to execution | ||
| order). |
Contributor
There was a problem hiding this comment.
I assume that in the upcoming PR you mentioned in person, this whole paragraph goes away to be replaced by something that says "we only model submodule imports adding the submodule attribute for global-scope submodule imports"
Comment on lines
+17
to
+23
| - **first from first serve**: only the *first* `from..import` in an `__init__.py(i)` that imports a | ||
| particular direct submodule of the current package introduces that submodule as a local. | ||
| Subsequent imports of the submodule will not introduce that local. This reflects the fact that | ||
| in actual python only the first import of a submodule (in the entire execution of the program) | ||
| introduces it as an attribute of the package. By "first" we mean "the first time in this scope | ||
| (or any parent scope)". This pairs well with the fact that we are specifically introducing a | ||
| local (as long as you don't accidentally shadow or overwrite the local). |
Contributor
There was a problem hiding this comment.
I like this paragraph, very clear. (Last sentence I also expect to go away in a future PR.)
|
|
||
| - **dot re-exports**: `from . import a` in an `__init__.pyi` is considered a re-export of `a` | ||
| (equivalent to `from . import a as a`). This is required to properly handle many stubs in the | ||
| wild. Currently it must be *exactly* `from . import ...`. |
Contributor
There was a problem hiding this comment.
And it sounds like you have a PR in the works that will also eliminate the last sentence here?
crates/ty_python_semantic/resources/mdtest/import/nonstandard_conventions.md
Outdated
Show resolved
Hide resolved
Comment on lines
920
to
924
| ## LHS `from` Imports In Functions (Non-Stub Check) | ||
|
|
||
| If a `from` import occurs in a function, LHS symbols should only be visible in that function. This | ||
| very blatantly is not runtime-accurate, but exists to try to force you to write "obviously | ||
| deterministically correct" imports instead of relying on execution order. |
Contributor
There was a problem hiding this comment.
This should change in an upcoming PR.
Comment on lines
+5554
to
+5556
| if !self.is_reachable(import_from) { | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
I assume this was caught be ecosystem? Do we have any test covering this case?
Contributor
Author
There was a problem hiding this comment.
No this is copy-paste from infer_from_import_definition, I have no insights. I just assumed it was a common "don't do diagnostics about dead code" idiom.
Contributor
There was a problem hiding this comment.
Yeah that's exactly what it is, makes sense that it would just be copied over. Don't even feel strongly about having a test for it, but it seems like a good idea to avoid future regression?
dcreager
added a commit
that referenced
this pull request
Nov 11, 2025
* origin/main: (38 commits) [ty] Make implicit submodule imports only occur in global scope (#21370) [ty] introduce local variables for `from` imports of submodules in `__init__.py(i)` (#21173) [`ruff`] Ignore `str()` when not used for simple conversion (`RUF065`) (#21330) [ty] implement `typing.NewType` by adding `Type::NewTypeInstance` [ty] supress inlay hints for `+1` and `-1` (#21368) [ty] Use type context for inference of generic constructors (#20933) [ty] Improve generic call expression inference (#21210) [ty] supress some trivial expr inlay hints (#21367) [`configuration`] Fix unclear error messages for line-length values exceeding `u16::MAX` (#21329) [ty] Fix incorrect inference of `enum.auto()` for enums with non-`int` mixins, and imprecise inference of `enum.auto()` for single-member enums (#20541) [`refurb`] Detect empty f-strings (`FURB105`) (#21348) [ty] provide `import` completion when in `from <name> <name>` statement (#21291) [ty] elide redundant inlay hints for function args (#21365) Fix syntax error false positive on alternative `match` patterns (#21362) Add a new "Opening a PR" section to the contribution guide (#21298) [`flake8-simplify`] Fix SIM222 false positive for `tuple(generator) or None` (`SIM222`) (#21187) Rebuild ruff binary instead of sharing it across jobs (#21361) [ty] Fix `--exclude` and `src.exclude` merging (#21341) [ty] Add support for properties that return `Self` (#21335) Add upstream linter URL to `ruff linter --output-format=json` (#21316) ...
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.
This rips out the previous implementation in favour of a new implementation with 3 rules:
froms are locals: a
from..importcan only define locals, it does not have globalside-effects. Specifically any submodule attribute
athat's implicitly introduced by eitherfrom .a import borfrom . import a as b(in an__init__.py(i)) is a local and not aglobal. If you do such an import at the top of a file you won't notice this. However if you do
such an import in a function, that means it will only be function-scoped (so you'll need to do
it in every function that wants to access it, making your code less sensitive to execution
order).
first from first serve: only the first
from..importin an__init__.py(i)that imports aparticular direct submodule of the current package introduces that submodule as a local.
Subsequent imports of the submodule will not introduce that local. This reflects the fact that
in actual python only the first import of a submodule (in the entire execution of the program)
introduces it as an attribute of the package. By "first" we mean "the first time in this scope
(or any parent scope)". This pairs well with the fact that we are specifically introducing a
local (as long as you don't accidentally shadow or overwrite the local).
dot re-exports:
from . import ain an__init__.pyiis considered a re-export ofa(equivalent to
from . import a as a). This is required to properly handle many stubs in thewild. Currently it must be exactly
from . import ....This implementation is intentionally limited/conservative (notably, often requiring a from import to be relative). I'm going to file a ton of followups for improvements so that their impact can be evaluated separately.
Fixes astral-sh/ty#133