-
Notifications
You must be signed in to change notification settings - Fork 767
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
The auto-import logic is not picking the shortest import path for symbols #774
Comments
Any chance you could try out the insiders build which has indexing enabled to see if that changes the import that's presented? #623 |
Thanks for the response @jakebailey ! I just enabled insiders, it was downloaded and I was asked to reload the editor, which I did. Now the completion system doesn't suggest anything for I get suggestions for other symbols with a name that starts with With the same snippet, I get: |
If you enable trace logging, do you see the indexer running and completing by the time you perform this completion? Curious that it doesn't show. @heejaechang can likely take a look when he's back from his break. |
I think I found how to do it and tried it, it seems the indexing is running and completing before I trigger completion. Here's the full log for the
|
it is happening due to FastApi/__ init __ .py doesn't define __ all __ . to reduce number of items to suggest, for regular py file, we only suggest symbols defined in __ all __ for now. @savannahostrowski @judej we might want to add import alias defined as "import * as alias" in index if __ all __ doesn't exist in __ init __ .py file? |
Thanks for replying @heejaechang ! I don't declare I prefer to declare explicit |
For reference, this is not a recommendation to use We won't be able to use only "X as X" to determine what to offer, as that'll miss anything actually declared in the file, I think, so we'll have to take a look to see how to handle this. |
FastAPI is a "py.typed" package. I think the indexer should change its behavior for such packages and follow the rules layed out in the typed library guidance. This guidance indicates specifically which symbols are considered public and which are not. For non-"py.typed" packages, it makes sense for the indexer to use more conservative heuristics to avoid adding lots of garbage to the completion suggestion list. |
We've identified a bug in one of the performance optimizations for |
This issue has been fixed in version 2021.4.0, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/main/CHANGELOG.md#202140-7-april-2021 |
@jakebailey I tried out the latest versions and the original issue still persists. The only way I've been able to resolve it is to move all the symbols directly into Can you elaborate on the fix and what (if any) changes should be made to libraries in order to have the auto-import functionality generate imports from |
Are you testing with fastapi or another project? This bug seemed to be specific to fastapi's layout, so your issue may be worth a new issue with a reproducer we can test and fix. |
Just to double check, do you have indexing explicitly enabled? |
Right, thanks. This bug is fixed with indexing, but we're not yet ready to get that enabled for everyone yet. Insiders sometimes has it enabled, but we are currently working on making sure it won't cause a big perf regression (so it isn't enabled right now to help us diff versions). |
Environment data
Expected behaviour
I would expect the auto-import feature to pick the shortest import path for a symbol, or at least offer it as one of the alternatives, rather than only the source file where that symbol is defined.
For example, take this snippet:
After triggering auto-completion for
Query
, I would want it to suggest to import it as:Or at least include that in the possible locations to import.
Actual behaviour
The auto-import feature is only suggesting the import path that points to the source file where the symbol is created.
So it suggests importing:
Logs
I think logs are not relevant here, but let me know.
Code Snippet / Additional information
Code snippet and screenshot provided above in context.
I recently released FastAPI
0.63.0
with support formypy --strict
, including re-exports in the source files for the symbols I want re-exported.This is a continuation of this comment: #222 (comment) in a previous issue.
The text was updated successfully, but these errors were encountered: