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

🐛 Recurse into dataclass when encoding #4454

Closed

Conversation

ConorStevenson
Copy link

@ConorStevenson ConorStevenson commented Jan 20, 2022

Context:
PR #3576 added support for dataclasses as response objects.

Problem:
json_encoder doesn't recurse into dataclasses as it does for pydantic models, dictionaries, and other collections. Custom encoders aren't applied to attributes on dataclasses.

Solution:
Recurse into dataclasses when encoding. Also change dataclass tests to use dataclasses rather than pydantic models.

Example:

from datetime import datetime
from fastapi import FastAPI
import dataclasses
import pydantic

app = FastAPI()

@pydantic.dataclasses.dataclass
class PydanticDataclass:
    timestamp: datetime = datetime.now()

@dataclasses.dataclass
class ClassicDataclass:
    timestamp: datetime = datetime.now()

@app.get("/pydantic", response_model=PydanticDataclass)
async def pydantic_dataclass():
    return PydanticDataclass()


@app.get("/classic", response_model=ClassicDataclass)
async def classic_dataclass():
    return ClassicDataclass()

$ curl localhost:8000/pydantic
-> {"timestamp":"2022-01-24T20:51:42.316967"}
$ curl localhost:8000/classic
-> TypeError: Object of type datetime is not JSON serializable

@himbeles
Copy link
Contributor

See also my existing (nearly identical) PR #3607 and issue #3608 from last year. No response until now unfortunately.

Copy link

@don4of4 don4of4 left a comment

Choose a reason for hiding this comment

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

Looks good -- encountered this issue in our projects as well.

@himbeles
Copy link
Contributor

Shouldn’t the keyword arguments of jsonable_encoder also be passed on into the recursion? See also #3607.

@ConorStevenson
Copy link
Author

Shouldn’t the keyword arguments of jsonable_encoder also be passed on into the recursion? See also #3607.

Yep, updated it now. Sorry to have duplicated your existing PR!

@himbeles
Copy link
Contributor

himbeles commented Feb 1, 2022

No problem, just hope it gets merged :)

This makes the behaviour of include/exclude easier to see
@himbeles
Copy link
Contributor

@don4of4 Thanks for the review. Do you see any chances, that this PR will be merged in the near future?

@himbeles
Copy link
Contributor

My PR #3607 got merged. I think you can close this one now! 🚀

@tiangolo
Copy link
Owner

tiangolo commented Sep 4, 2022

Thanks for the effort @ConorStevenson! ☕

It seems this was solved by #3607, right? Given that, I'll close this one.

But if you feel there's something else that needs to be done, feel free to create a new PR.

Thanks! 😄

@tiangolo tiangolo closed this Sep 4, 2022
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.

None yet

4 participants