Pylance support for FastAPI

debonte commented 1 year ago

Thoughts on how Pylance could improve the FastAPI development experience.

Help with path parameters

Link the {foo} formatting tokens in FastAPI.get/FastAPI.put/etc paths with their corresponding function parameters. For example, given the code below from the example on their homepage, we could:

While modifying the path string of the decorator, if the user types {, provide completions for the names of any existing parameters on the decorated function.
While adding paramters to the function, provide completions that match the {foo} formatting tokens in the decorator's path. The user can add parameter than don't match the formatting tokens (ex. q on read_item), but in most cases they will want to have parameters that match the tokens.
Include the formatting tokens in rename, find all references, etc. For example, renaming item_id in read_item would rename the parameter, it's usage, and {item_id} in the decorator path. Not sure if it's worth supporting initiating the rename from the {item_id} token.

@app.get("/items/{item_id}")
def read_item(item_id: int, q: Union[str, None] = None):
    return {"item_id": item_id, "q": q}

@app.put("/items/{item_id}")
def update_item(item_id: int, item: Item):
    return {"item_name": item.name, "item_id": item_id}

Convertors

Starlette, which FastAPI is based on, uses convertor objects to convert parameters passed in a URL (which are all strings) to the type expected by the application (ex. integer). FastAPI takes care of this under the hood in most cases (based on function parameter type annotations), but in some cases this isn't possible. One case they call out in their tutorial is paths, where you want everything after a certain point in the URL to be given to you as a single string, including slashes. Starlette also supports custom convertors. See https://www.starlette.io/routing/.

Depending on how common convertor usage is, we might want to provide convertor types as completions after : in a path.

@app.get("/files/{file_path:path}")
async def read_file(file_path: str):
    return {"file_path": file_path}

Templates

FastAPI users would benefit from language server support for jinja files. The docs cover templating here.

Diagnostics

We would prefer to not emit Pylance-specific diagnostics since that would mean Pylance's diagnostics and Pyright CI diagnostics would differ, but I'm listing some ideas in case we ever find a way to support library-specific diagnostics:

Two functions decorated with the same operation and path is an error. The second one will never be called. See https://fastapi.tiangolo.com/tutorial/path-params/#order-matters
Similarly, if two functions are decorated with the same operation and their paths are the same except for a fixed string vs. a formatting token (ex. /users/me and /users/{user_id}), they must be declared with the fixed string version first. See https://fastapi.tiangolo.com/tutorial/path-params/#order-matters
It's unfortunate that the value of Query.default isn't checked against the annotated type of the parameter it's used on.
```
def read_items(q: str = Query(default=1)):
```
Also, you can pass non-sensical parameters to Query (ex. max_length for an int parameter). In this case you'll get a runtime error though: "ValueError: On field "q" the following field constraints are set but not enforced: max_length."
Are Query, Path, Body, etc interchangeable? If not, would it be useful to detect when Query is incorrectly used on a path parameter or Path is incorrectly used on a query parameter?

Other thoughts

FastAPI has built-in support for Pydantic models. Our Pydantic support should be decent by virtue of them using dataclass_transform, but it would be worth checking for any pain points there.
FastAPI doesn't have a built-in ORM like Django does. They seem to assume that you'll use Alembic for migrations. If that's super-common we might consider adding Alembic migration commands.

debonte commented 1 year ago

@tiangolo, if you have any thoughts on how Pylance could improve the FastAPI development experience, we'd love to hear them!

tiangolo commented 1 year ago

This is awesome! Thank you! And thanks for pinging me.

The points about path parameters are great, and that's one of the very few cases that would need more custom support than standard language syntax.

Path parameter convertors would be cool, but it's probably not used much, so I would prioritize other things first.

Soon I'll support and recommend parameters and dependencies in Annotated, that will help with and simplify default values as standard Python support will do it.

def read_items(q: Annotated[str, Query()] = 1):

FastAPI's recommended ORM if SQLModel, I built it for it, I have to update the docs. One of the features I'll add is wrappers for Alembic commands (the whole thing is a wrapper around SQLAlchemy). It already used dataclass_transform, and when building it I did all the typing tricks to ensure it would work great in VS Code (and PyCharm too).

Here's one thing that is not really publicly known yet, but is coming in the next few weeks, and you could potentially have a very important role in it.

I'm gonna document the whole user API (the FastAPI code), but I'm not gonna use one of the several docstring formats but this new idea I've been working on, based on Python type hints and standard Python syntax. https://github.com/tiangolo/fastapi/blob/typing-doc/typing_doc.md

As a note, that's gonna be rendered on the docs using an extension to MkDocstrings that @pawamoy has been working on.

I suspect this might be the thing you could do with the biggest impact in FastAPI support, maybe even bigger than the previous points, because everyone would see those tooltips everywhere in their codebase.

The information could be shown in tooltips, autocompletion, param info when writing arguments, etc.

The design is using standard Python syntax, so no need for a new parser, just extract data from the syntax tree, which is already associated with each param.

The tech document pre-standard is very much inspired by dataclass_transform, and it's all designed to make it very easy to support. Still, if there's something that would make the implementation difficult, or that could improve it, I would appreciate your feedback.

Note: I get thousands of GitHub notifications and I could miss this, but this is important to me, if I'm not replying about it there's a chance I didn't see it, please ping me on Twitter or email.

erictraut commented 1 year ago

@tiangolo, it looks like you're planning to move forward with your proposal to use Annotated for parameter documentation. Consistent with the feedback I gave you in email, I think this proposal will meet significant resistance from the broader typing community. It was discussed in a recent typing meetup, and the consensus was that it was too verbose and made function signatures difficult for human readers to parse. If you want to move forward with this, I think you're going to need to do more convincing. Pyright (and other type checkers / language servers) are unlikely to adopt this if it's not standardized.

Jelle Zijlstra has created an overlapping PEP 702 for marking functions and classes as deprecated, and that proposal was well received. Pyright already has support for the draft PEP.

tiangolo commented 1 year ago

Thanks Eric. Yeah, it was a bit sad I didn't get the chance to present it properly with all the arguments and explanations before that, but anyway. This would not benefit mainly typing, but maintainers of docstrings and documentation. I have received mainly positive feedback from developers, library authors and core developers, the main criticism has been this one you collected.

About verbosity and human readers, type annotations themselves could be considered as affecting that, signatures are more verbose and take a bit longer to read when there are types. A less verbose alternative was to have types in docstrings, or a separate file. But the information carried in types is closely related to those parameters, it made sense for that information to be closely together, it improves consistency, reduces duplication, avoids bugs, it's the same standard syntax so giving support for maintainers using it doesn't require special effort, etc. Documentation, currently only in docstrings, has those same properties, drawbacks, and benefits as type annotations.

That's also why I wanted to work more on it and have a working and tested prototype, in an early-adopters version, the same way you did with dataclass-transform. And show how it would be useful.

I also don't expect the common developers to use it in their apps, the same way I don't see them using docstrings a lot. This would be used mainly by library authors. And from those is that I've received mainly positive feedback.

In any case, VS Code Python could at least make sure it doesn't show the Annotated part in tooltips, as that's intentionally made to carry metainformation not related to the type. I guess the simplest and best generic UI, for Python in general (independent of my thing) would be to show only the type in tooltips (the first parameter to Annotated).

About Jelle, yep, I talked to him and he told me I should continue with my idea separately as it's more ambitious, and he will continue with @deprecated on his own.

erictraut commented 1 year ago

Thanks for the additional context.

VS Code Python could at least make sure it doesn't show the Annotated part in tooltips, as that's intentionally made to carry metainformation not related to the type

Are you referring to Pylance or some other language server? Pylance doesn't show the Annotated parts in hover text (tooltips) today. If you are seeing the Annotated part appear, please file a bug with repro steps, and we'll fix that.

tiangolo commented 1 year ago

Are you referring to Pylance or some other language server? Pylance doesn't show the Annotated parts in hover text (tooltips) today. If you are seeing the Annotated part appear, please file a bug with repro steps, and we'll fix that.

Ah, that's great! Sorry I didn't double-check before writing, I did it on the phone. :grimacing:

If Pylance already omits the Annotated stuff, that's perfect. :ok_hand:

Re my proposal: I want to test it out in FastAPI and friends as you did with dataclass-transform, before pushing much for the standard, that would probably help finding problems, fine-tuning it a bit more, assessing the potential benefits (and drawbacks) better, adding and removing stuff, etc. I would hope to present it at the Typing Summit on PyCon US (if I get admitted :sweat_smile:). I would hope before, but I tend to overestimate how much I can do, so we'll see.

hmc-cs-mdrissi commented 1 year ago

Including documentation inside of Annotated is practice I’ve been doing a while (year and half) for an internal library similar to data classes. I’ve even made dataclasses like decorator to enforce that any user that uses it by default must have every parameter documented inside annotated. That’s probably stricter then you intend but I can be more picky on rules like this for a company internal library. The decorator is mostly used for defining serialization/deserialization for python classes (similar in spirit to cattrs/pydantic).

My primary motivation was making runtime introspection of parameter documentation easy and would allow simple sphinx plugin. The sphinx plugin is now theoretical but I’ve been pretty happy with side effect so far of forcing people to document arguments of any configs.

I’d be happy to discuss experience/usage more offline.

debonte commented 1 year ago

pawamoy commented 1 year ago

@debonte these are all 404. Is this a private repository? In any case let me know and I'll remove my comment :slightly_smiling_face:

debonte commented 1 year ago

@pawamoy, yes, pyrx is a private repository used by the Pylance team. Sorry for the confusion. I just wanted to link together our public issues and internal issues related to FastAPI.

tiangolo commented 8 months ago

Hello @debonte!

I wrote PEP 727, and there's been a long discussion about it.

I implemented it in FastAPI, now each FastAPI parameter has documentation inside of Annotated using typing_extensions.Doc. That's what powers the new reference: https://fastapi.tiangolo.com/reference/.

Eric is mostly in disagreement with the proposal. Nevertheless, he mentioned in the forum that it would be viable for me to consult with you if you would consider implementing support for rendering documentation for the parameters in FastAPI functions and classes as a (probably experimental) Pylance feature, rather than a Pyright feature, so, as suggested by him, I came here to ask for your opinion from the Pylance team.

debonte commented 8 months ago

Hi @tiangolo!

When we add experimental support for a draft PEP, it's generally with the goal of helping the community make a decision on whether that PEP should be accepted. Given that the objections in the discussion on discuss.python.org are centered around the experience of package authors rather than end-users, we don't believe that adding rendering support will help with this decision making process on PEP 727. Therefore, we feel that this would not be a good use of our limited resources. Should the PEP be accepted, we will of course consider adding support for it.

tiangolo commented 4 months ago

Thanks for the response @debonte, and sorry for my delay in replying back.

Totally get it that you wouldn't want to add support for this as it could be thought of as pressure into accepting the PEP. And if it was rejected, it would be a lost time/resources investment.

Alternative `annotated-doc`

One of the suggested alternatives was to build an independent package, not part of typing or typing-extensions, e.g. annotated-doc, that would provide this same Doc object.

The main disadvantage I see now for that would be that as it's not part of typing, people would have to explicitly install it and depend on it to use it. And I had the idea that it would be more difficult for you and other providers to support something like that if it was not a standard.

The main advantage I see is that I would be able to use that on FastAPI, SQLModel, Typer, Asyncer, and keep using it even if the PEP is rejected. And others could do that as well. And as I see that you can support some of the other conventions to document things (Sphinx, numpy, Google), maybe it could be acceptable to support this thing this way, independent of any PEPs.

Another benefit would be that maybe it would relax any pressure into accepting the PEP, as people would be able to use it outside of the standards and test it out before making it "official". I had originally thought the best and right approach was to try and create the PEP and make it all very official and part of the standards, but after all those discussions I see it could be interpreted as pressure into accepting it.

Maybe making this an external package independent of PEPs could work, at least for some time, while I and a few others test it out. Similar to how ASGI is not a PEP, but has gained a lot of adoption, etc.

Would Pylance consider `annotated-doc`

Before going that route, I wanted to check if adding support for something like that would be more or less acceptable as something you would consider implementing support for.

In this scenario, you would not be implementing support for a draft PEP, that if accepted could be used by anyone, but if rejected would be more or less lost work.

Instead, in this scenario, you would be implementing support specifically for FastAPI, SQLModel, Typer, Asyncer, and a small few others that decide to try it. So, the userbase of this feature would not be as big as all Python users, would only be users of FastAPI and friends. But implementing it would keep serving those same users without the possibility of losing that work if a PEP is rejected.

I guess the decision would be similar to adding support or not for Numpy docstrings, scaling the user base (and estimated user growth) to FastAPI instead of Numpy.

Is that something you would consider more viable? Or would it be even less viable?

Additional uses of `annotated-doc`

If I can use this pattern/idea, my intention is to also support it for generating documentation for those parameters in apps in FastAPI and Typer. So that users could define parameters with Doc and FastAPI and Typer would extract that for the API docs UI and the CLI help menus.

This is not particularly related to this discussion, just as general additional context.

debonte commented 4 months ago

I guess the decision would be similar to adding support or not for Numpy docstrings, scaling the user base (and estimated user growth) to FastAPI instead of Numpy.

@tiangolo, that's fair. I'll take a look at how expensive this would be for us to implement.

debonte commented 2 months ago

Hi @tiangolo,

Sorry for the delay.

I see that you can support some of the other conventions to document things (Sphinx, numpy, Google), maybe it could be acceptable to support this thing this way, independent of any PEPs.

Our support for these other conventions effectively boils down to transforming docstring text from one format to another (markdown). Adding support for PEP 727-style docstrings (esp. function doc strings) would be significantly more complex, and we feel the benefit it would provide isn't worth the risk and maintenance costs it would introduce.

Without the PEP being accepted I don't see us investing in this.

tiangolo commented 2 months ago

Thanks for the feedback @debonte! In that case, I won't try that route of another package for now. I'll just keep the typing_extensions version for now in my packages. And we'll see how things evolve over time with the PEP, etc. Thanks!

microsoft / pylance-release