Support resolving via "from ... import ..." in normal python modules #118
Conversation
| import_source = ModulePath(tuple(star_import.split("."))) | ||
| if import_source != module_name: | ||
| # Try to resolve stub from star import | ||
| resolved_name = self.get_name(import_source, name) |
There was a problem hiding this comment.
It can happen that a name imported from one module, is also imported in that module. So this needs to be recursive.
typeshed_client/finder.py
Outdated
| @@ -366,7 +373,7 @@ def find_typeshed() -> Path: | |||
| return importlib_resources.files("typeshed_client") / "typeshed" | |||
|
|
|||
|
|
|||
| def parse_stub_file(path: Path) -> ast.Module: | |||
| def ast_parse_file(path: Path) -> ast.Module: | |||
There was a problem hiding this comment.
Let's keep the name parse_stub_file as an alias in case any users of the library are using the old name, since it's public.
There was a problem hiding this comment.
Certainly, I will change it back.
There was a problem hiding this comment.
I changed it back. But, did you mean to keep the new name but just do parse_stub_file = ast_parse_file for backward compatibility?
| @@ -34,7 +34,32 @@ def get_module(self, module_name: ModulePath) -> "Module": | |||
|
|
|||
| def get_name(self, module_name: ModulePath, name: str) -> ResolvedName: | |||
| module = self.get_module(module_name) | |||
There was a problem hiding this comment.
I think the way I'd want to do this is more in get_module. Currently, that will only look for pyi files, but it should look for py files too as a fallback. Basically, the library should follow https://typing.python.org/en/latest/spec/distributing.html#import-resolution-ordering and look at all the places data can come from, both pyi and py files.
Then when we parse the module, we should probably do it in a different 'mode' than we do parsing .pyi files, because when parsing .py files we need to be more forgiving about unrecognized AST nodes.
We may also need to put this new functionality behind an off-by-default flag so we don't interfere with existing users that look for .py files separately already.
There was a problem hiding this comment.
We may also need to put this new functionality behind an off-by-default flag so we don't interfere with existing users that look for .py files separately already.
Done.
I think the way I'd want to do this is more in
get_module. Currently, that will only look for pyi files, but it should look for py files too as a fallback. Basically, the library should follow https://typing.python.org/en/latest/spec/distributing.html#import-resolution-ordering and look at all the places data can come from, both pyi and py files.
I looked at this but unfortunately did not understand. After get_module is called, it is not known yet if name is defined in that module. This is only known after module.get_name(name, self) is called. Please look again and explain a bit more how you think it should be changed.
There was a problem hiding this comment.
Then when we parse the module, we should probably do it in a different 'mode' than we do parsing .pyi files, because when parsing .py files we need to be more forgiving about unrecognized AST nodes.
This I am already doing with the _PyImportedSourcesExtractor visitor I implemented. Does this look okay?
|
@JelleZijlstra I would work on improvements to this, but I would need your input. If you don't have time for this, or you are unsure about adding this feature, please let me know. |
|
Sorry, I am hoping to get back to it soon! |
Fixes #117
This is just some initial idea. If it seems to be in the right track I can think about how to write unit tests. It is the first time I look at the code in this repo. Not sure if there are details I completely missed to consider.