-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Support implicit imports of submodules in __init__.pyi
#20855
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.
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
base: main
Are you sure you want to change the base?
Conversation
This is a second take at the implicit imports approach, allowing `from . import submodule` in an `__init__.pyi` to create the `mypackage.submodule` attribute everyhere. This implementation operates inside of the available_submodule_attributes subsystem instead of as a re-export rule. The upside of this is we are no longer purely syntactic, and absolute from imports that happen to target submodules work (an intentional discussed deviation from pyright which demands a relative from import). Also we don't re-export functions or classes. The downside(?) of this is star imports no longer see these attributes (this may be either good or bad. I believe it's not a huge lift to make it work with star imports but it's some non-trivial reworking). I've also intentionally made `import mypackage.submodule` not trigger this rule although it's trivial to change that. I've tried to cover as many relevant cases as possible for discussion in the new test file I've added (there are some random overlaps with existing tests but trying to add them piecemeal felt confusing and weird, so I just made a dedicated file for this extension to the rules). Fixes #133
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
CodSpeed Performance ReportMerging #20855 will degrade performances by 11.02%Comparing Summary
Benchmarks breakdown
|
|
__init__.pyi
__init__.pyi
|
Lint rule | Added | Removed | Changed |
---|---|---|---|
unresolved-attribute |
0 | 61 | 1 |
invalid-argument-type |
12 | 0 | 0 |
call-non-callable |
3 | 0 | 0 |
possibly-missing-attribute |
1 | 0 | 0 |
Total | 16 | 61 | 1 |
/// The set of modules that are imported anywhere within this file. | ||
maybe_imported_modules: Arc<FxHashSet<(u32, Option<String>, String)>>, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this different from imported_modules
?
I also suggest using a named struct here. It's non obvious what the semantic meaning of the tuple fields are.
It might be worth using Name
instead of String
, to reduce the amount of heap allocations (or use ModuleName
?)
.iter() | ||
// Throw out the result if the import is not a module (e.g. it's a function/class) | ||
// So that `from a.b import myfunction` is not considered an import of `a.b` | ||
.filter(|submodule| self.resolve_submodule(db, submodule).is_some()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we move this check into maybe_imported_relative_submodules_of_stub_package
, given that maybe_imported_relative_submodules_of_stub_package
is cached? Is it worth caching maybe_imported_relative_submodules_of_stub_package
?
This is a second take at the implicit imports approach, allowing
from . import submodule
in an__init__.pyi
to create themypackage.submodule
attribute everyhere.This implementation operates inside of the available_submodule_attributes subsystem instead of as a re-export rule.
The upside of this is we are no longer purely syntactic, and absolute from imports that happen to target submodules work (an intentional discussed deviation from pyright which demands a relative from import). Also we don't re-export functions or classes.
The downside(?) of this is star imports no longer see these attributes (this may be either good or bad. I believe it's not a huge lift to make it work with star imports but it's some non-trivial reworking).
I've also intentionally made
import mypackage.submodule
not trigger this rule although it's trivial to change that.I've tried to cover as many relevant cases as possible for discussion in the new test file I've added (there are some random overlaps with existing tests but trying to add them piecemeal felt confusing and weird, so I just made a dedicated file for this extension to the rules).
Fixes #133
Summary
Test Plan