Skip to content
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

In the absence of stub files, auto-import should use better heuristics for picking preferred import path #222

Closed
erictraut opened this issue Aug 9, 2020 · 5 comments
Labels
enhancement New feature or request fixed in next version (main) A fix has been implemented and will appear in an upcoming version needs spec

Comments

@erictraut
Copy link
Contributor

This suggestion comes from @tiangolo and is copied from microsoft/pyright#922.

The auto-import feature is not as smart as it could be about choosing the best import path for a symbol. With stub files, a library author can (but doesn't need to) specify a single export point for a symbol. For libraries that are missing stub files, a symbol is often imported and then re-exported by many submodules.

The auto-import feature should generally choose the shortest import path — i.e. the highest-level module that re-exports it.

@github-actions github-actions bot added the triage label Aug 9, 2020
@savannahostrowski savannahostrowski added enhancement New feature or request needs spec and removed triage labels Aug 10, 2020
@heejaechang heejaechang added the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Sep 2, 2020
@heejaechang
Copy link
Contributor

now we handle this situation. for import alias (re-exported by stub files), we only show one that is the shortest path (smallest ".") in completion.

@jakebailey
Copy link
Member

This issue has been fixed in version 2020.9.0, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/master/CHANGELOG.md#202090-3-september-2020

@tiangolo
Copy link

Hey all! Thanks for this. 🚀

(And as I'm writing this on Dec 25, Merry Christmas/happy holidays to you all! 🎄 🤶 )

I recently released a new version of FastAPI with support for mypy --strict, with explicit symbol re-export and integrated type annotations in the source files (instead of additional/external .pyi files).

This was all after some email conversations with @erictraut finding the best ways to support type analysis, and after reading the guidelines in https://github.com/microsoft/pyright/blob/master/docs/typed-libraries.md

Now, when I'm on a block like:

from fastapi import 

...and I trigger autocompletion there, Pylance/Pyright now can correctly detect that there are symbols available to import from the top level, like Query. And the final result is:

from fastapi import Query

That's great! 🎉 👏


But then when using auto-import, it is not behaving as I would imagine.

With a snippet like:

from fastapi import FastAPI

app = FastAPI()

@app.get("/")
def main(name: str = Query()):
    pass

...if I trigger autocompletion/auto-import for Query, it suggests to import from the source of that function Query, at fastapi.param_functions.

Selection_215

I would expect it to suggest:

from fastapi import Query

Environment data

  • Language Server version: Pylance language server 2020.12.2 (pyright 6feebac9)
  • OS and version: Ubuntu 18.04, VS Code 1.51.1
  • Python version (& distribution if applicable, e.g. Anaconda): 3.7 in a venv with a fresh FastAPI installation.

I imagine this is the right place to comment about this, but let me know if it should be in microsoft/pyright#922 or if I should create a new issue.

Thanks! 🍰 ☕

@erictraut
Copy link
Contributor Author

Thanks @tiangolo for the issue. I think this one is different from the one that kicked off this thread. Could you please open a new issue for pylance-release? I just reviewed the latest version of __init__.py checked in to the fastapi repo, and it looks correct to me, so this must be a bug in pylance's auto-import logic. It should prefer the shorter import path.

@tiangolo
Copy link

Thanks for the speedy reply and feedback @erictraut ! 🤓

I just created a new issue here: #774

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed in next version (main) A fix has been implemented and will appear in an upcoming version needs spec
Projects
None yet
Development

No branches or pull requests

5 participants