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

Let library authors specify preferred way to auto-import #922

Closed
tiangolo opened this issue Aug 8, 2020 · 6 comments
Closed

Let library authors specify preferred way to auto-import #922

tiangolo opened this issue Aug 8, 2020 · 6 comments
Labels
enhancement request New feature or request

Comments

@tiangolo
Copy link

tiangolo commented Aug 8, 2020

Is your feature request related to a problem? Please describe.

I love auto-import, I use it everywhere. 🚀

But I think it resolves the object that I'm importing to the exact module where it was declared, and in some cases, the library's "recommended" place from where to import something is not the exact module that object is created at.

For example, take this incomplete FastAPI example:

from fastapi import FastAPI

app = FastAPI()


@app.get("/")
def read_main(q: str = Query(None)):
    return {"message": "Hello World"}

Triggering auto-import for Query suggests from fastapi.param_functions import Query, but the "ideal" would be from fastapi import Query.

Selection_089

The FastAPI ideal would be:

from fastapi import FastAPI, Query

app = FastAPI()


@app.get("/")
def read_main(q: str = Query(None)):
    return {"message": "Hello World"}

As another example, this (incomplete but sufficient) SQLAlchemy snippet:

from sqlalchemy import Column


class User:
    name = Column(String)

Triggering auto-import for String suggests from sqlalchemy.sql.sqltypes import String.

Selection_091

But the ideal would be from sqlalchemy import String as in:

from sqlalchemy import Column, String


class User:
    name = Column(String)

Describe the solution you'd like

The simplest and first option I would think of is to change the logic to suggest importing from the most top-level module where the import is available. So, instead of suggesting from fastapi.param_functions import Query it would suggest from fastapi import Query.

I see that for type stubs, declaring an import x as x is required to mark it as exported/importable: https://mypy.readthedocs.io/en/stable/command_line.html#cmdoption-mypy-no-implicit-reexport

So that could be a way to let library authors show their preferred import location, and is more or less "standard" in some way (it's the mechanism used by type stubs).


The next thing I would like (that I imagine has a lower chance of happening) is to include auto-import suggestions even if they are created in another package (probably when they are imported in the library as import x from x).

As a clear example, FastAPI imports and re-exports Request from Starlette, so it's possible (and suggested) to do:

from fastapi import FastAPI, Request

app = FastAPI()


@app.get("/")
def read_main(request: Request):
    return {"message": "Hello World"}

but with this incomplete example:

from fastapi import FastAPI

app = FastAPI()


@app.get("/")
def read_main(request: Request):
    return {"message": "Hello World"}

triggering auto-import for Request suggests from starlette.requests import Request.

Selection_092

Additional context

Another option I would see is to add more settings for users to specify preferences, but I think that would be less than ideal.

I know Pyright wants to focus on full typing and standards compatibility, so I think the stubs option of using the import x as x in the library could be a good option to let library authors suggest their preferred import location.

@tiangolo tiangolo added the enhancement request New feature or request label Aug 8, 2020
@erictraut
Copy link
Collaborator

Library authors should provide type stubs for their libraries. That's the correct solution to the problem you're posing, and the Python community is making a push to encourage the distribution of high-quality stubs with all popular libraries.

Type stubs, unlike ".py" files, unambiguously specify which imported symbols are re-exported. This gives library authors full control to express the proper import location for symbols.

@erictraut
Copy link
Collaborator

I agree that Pyright could employ better heuristics for choosing the preferred option when it is ambiguous. We're working on a significant update to the auto-import functionality, and that will be part of it.

@tiangolo
Copy link
Author

tiangolo commented Aug 8, 2020

In the case of FastAPI, it is fully type annotated and marked as providing its own types in code with a py.typed as per PEP 561, I'm not sure additional type stubs would work well, but maybe that will all improve with the refactor you mention. Awesome! 🚀

@erictraut
Copy link
Collaborator

Annotated source code cannot accurately and completely describe a library's public interface contract — at least with the mechanisms provided by the Python standard today. Library authors need to produce ".pyi" files to describe their public interfaces. Refer to this documentation for why type stub files are so important.

@erictraut
Copy link
Collaborator

@tiangolo, since you're a library developer yourself, I'd love to get your input on some ideas we're consider with pyright and type stubs. I'll connect with you directly on LInkedIn.

@erictraut
Copy link
Collaborator

I've created a new feature request in the pylance-release repo that captures the heart of your feature request. We're tracking all of the pylance auto-import bugs in that location. microsoft/pylance-release#222

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants