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

✨ Add support for function return type annotations to declare the response_model #1436

Merged
merged 9 commits into from
Jan 7, 2023

Conversation

uriyyo
Copy link
Contributor

@uriyyo uriyyo commented May 20, 2020

I love the idea of OpenAPI auto-generated schema and the FastAPI at all 😄

This is a feature request.

When I was working with FastAPI I thought that it will be a great idea to use function return type annotation as default response_model for an endpoint.

For instance, we have a simple application:

from pydantic import BaseModel

from fastapi import FastAPI


class Model(BaseModel):
    name: str


app = FastAPI()


@app.get("/", response_model=Model)
def endpoint():
    return Model(name="Yurii")

So with this PR, it can be rewritten to:

from pydantic import BaseModel

from fastapi import FastAPI


class Model(BaseModel):
    name: str


app = FastAPI()


@app.get("/")
def endpoint() -> Model:
    return Model(name="Yurii")

In case when response_model argument set and return type annotation present, then response_model will be used.

@codecov
Copy link

codecov bot commented May 20, 2020

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (685f402) compared to base (cf73051).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 685f402 differs from pull request most recent head fe9c95c. Consider uploading reports for the commit fe9c95c to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1436   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          540       541    +1     
  Lines        13969     13989   +20     
=========================================
+ Hits         13969     13989   +20     
Impacted Files Coverage Δ
fastapi/applications.py 100.00% <ø> (ø)
tests/test_reponse_set_reponse_code_empty.py 100.00% <ø> (ø)
fastapi/dependencies/utils.py 100.00% <100.00%> (ø)
fastapi/routing.py 100.00% <100.00%> (ø)
tests/test_response_model_as_return_annotation.py 100.00% <100.00%> (ø)
tests/test_datastructures.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@phy25
Copy link

phy25 commented May 20, 2020

Have you searched through the issue tracker before submitting this? I remembered there is a reason that this is not implemented.

@uriyyo
Copy link
Contributor Author

uriyyo commented May 20, 2020

It was discussed at #101

Use can use a return type annotation as response_model, but in a case when it leads to some type errors it will be replaced with response_model keyword argument at endpoint decorator.

@phy25 What do you think about such option?

@ghost
Copy link

ghost commented Jun 4, 2020

This was also discussed in #875, I ended up implementing the same in my codebase basically by creating a subclass of the APIRouter that I use to create all my routers.

@uriyyo
Copy link
Contributor Author

uriyyo commented Jun 4, 2020

@juhovh-aiven Thanks for the comment. I will do the same as you did.

I will close this PR. But I think that won't be the last PR regarding this feature😔

@github-actions
Copy link
Contributor

📝 Docs preview for commit 1d6161c at: https://63567e2f28003420c0eae5ff--fastapi.netlify.app

@uriyyo
Copy link
Contributor Author

uriyyo commented Oct 24, 2022

@tiangolo Should I also update docs?

@github-actions
Copy link
Contributor

📝 Docs preview for commit 8c4cf25 at: https://6356805d246dd92bb09291ac--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 685f402 at: https://6356824de6485c2a69a44560--fastapi.netlify.app

@cj
Copy link

cj commented Oct 27, 2022

I just ran into this myself, can't wait for this to get merged in! Great job @uriyyo 😄

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2022

📝 Docs preview for commit 01640c4 at: https://6366a6668c085d1c80d8798d--fastapi.netlify.app

Copy link
Contributor

@yezz123 yezz123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@uriyyo
Copy link
Contributor Author

uriyyo commented Nov 6, 2022

@tiangolo I would love to use this feature in my project. Is there any chance you can include it in the next release?

Comment on lines 27 to 29
@app.get("/valid3", response_model=ModelTwo)
def valid3() -> ModelOne:
return ModelTwo(surname="Test")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iudeen They actually different. On lines 22-44 there is no return type annotation but 27-29 has it.

@github-actions
Copy link
Contributor

📝 Docs preview for commit 3b3d039 at: https://639cc85ab5df5c0d9667060d--fastapi.netlify.app

@tiangolo tiangolo changed the title 🎉 Use function return type annotation as response_model ✨ Add support for function return type annotations to declare the response_model Jan 7, 2023
@tiangolo
Copy link
Member

tiangolo commented Jan 7, 2023

Thanks for the contribution @uriyyo! 🚀 🍰

And thanks for the discussion, everyone. ☕ 🍪

I added a couple of dozen tests to cover all the corner cases, interactions between return types and response_model, different types of declarations, data filtering, etc.

And I updated the docs to include all the information about this, how to use it, how and when to use response_model instead of the return type annotation, etc. 🤓

This will be available in FastAPI 0.89.0 released (most probably) in the next few hours. 🚀

@Tishka17
Copy link

Tishka17 commented Jan 9, 2023

I have created simple route:

class Resp(BaseModel):
    a: Optional[str] = Field(..., nullable=True)

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        print("init")


@app.get("/")
async def root() -> Resp:
    res = Resp(a=None)
    print("Res created", res)
    return res

And this log is not what I really expect to see:

INFO:     Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
init
Res created a=None
init

Probably it's ok for response_model case, but if I return model directly I do not expect it to be recreated by some reason internally.

Moreover, by that reason code is not working at all if I declare model this way

class Resp(BaseModel):
    a: Optional[str] = Field(..., nullable=True)

    class Config:
        fields = {'a': {'exclude': True}}

@iudeen
Copy link
Contributor

iudeen commented Jan 10, 2023

This seems to break in many cases. A lot of issues being reported on this.

JonasKs pushed a commit to JonasKs/fastapi that referenced this pull request Jan 12, 2023
…sponse_model` (fastapi#1436)

Co-authored-by: Sebastián Ramírez <tiangolo@gmail.com>
@peterbe
Copy link

peterbe commented Jan 26, 2023

Pardon my ignorance but what's the benefit of this?

Before:

@app.get("/", response_class=PlainTextResponse)
async def root():
    return "bla bla"

Swagger can infer that the response is going to be text/plain. I only have to specify what type of response it is once.

After

@app.get("/")
async def root() -> PlainTextResponse:
    return PlainTextResponse("bla bla")

Now Swagger defaults to thinking it's application/json and I have to type PlainTextResponse twice.

Yes, I'm a still a FastAPI newbie but I use it in a production project and was intrigued to see if this is something I should change to.

@tiangolo
Copy link
Member

tiangolo commented Feb 6, 2023

@Tishka17 the model is cloned to ensure that data is filtered, otherwise, if you return an instance of a subclass of that model (e.g. a subclass that includes a password field), if the class is not cloned and the data is created again, it would pass Pydantic validation by default, just for being a subclass. That's why it has to be recreated. It will probably change and improve in Pydantic v2. You can read more about why is that here: https://fastapi.tiangolo.com/tutorial/response-model/#return-type-and-data-filtering

But if you have more follow up questions/ideas, please create a new GitHub Discussion question.


@iudeen could you point me to those issues reported related to this? Maybe in Discord? (because I'll lose it in the GitHub notifications). I wanna check if there's something specific flawed about this approach or just several corner cases.

As FastAPI is used by thousands and thousands of developers, any change will probably break some corner cases. But if the change is an improvement for everyone then it should be okay (that's also why I release so granularly, so many releases, so that people can pin and very gradually upgrade if there are changes that become problematic for them).

But if there's something wrong with the approach or the implementation, or if there's one of those "corner cases" that is very, very common (not so "corner"), then it could deserve its own fix/workaround.


@peterbe This is related to response_model, not to response_class. And in your use case, keeping the code you had would probably be better (that's what I would do). If you have more questions, please create a new GitHub Discussion.

@iudeen
Copy link
Contributor

iudeen commented Feb 6, 2023

@tiangolo the issues were resolved in 0.89.1

baoliay2008 added a commit to baoliay2008/lccn_predictor that referenced this pull request Feb 16, 2023
given that FastAPI `response_model` now support return type annotation

see more details at:
1. fastapi/fastapi#101
2. fastapi/fastapi#1436
axel584 pushed a commit to axel584/fastapi that referenced this pull request Mar 5, 2023
…sponse_model` (fastapi#1436)

Co-authored-by: Sebastián Ramírez <tiangolo@gmail.com>
mahenzon pushed a commit to mahenzon/fastapi that referenced this pull request Mar 11, 2023
…sponse_model` (fastapi#1436)

Co-authored-by: Sebastián Ramírez <tiangolo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants