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

🐛 Fix jsonable_encoder for dataclasses with pydantic-compatible fields #3607

Merged
merged 6 commits into from Aug 26, 2022

Conversation

himbeles
Copy link
Contributor

@himbeles himbeles commented Jul 26, 2021

This PR fixes an issue (#3608) I encountered when returning dataclasses responses with pydantic-compatible (but incompatible with json encoder of Python standard library) fields, e.g. datetime objects or pathlib file paths.

This was achieved by adding a call to jsonable_encoder(dict) after converting the dataclass to a dict in jsonable_encoder.

For testing, I have added a datetime field to the Item class in tests/test_serialize_response_dataclass.py.
Without this PR, the last two tests in this file fail (test_no_response_model_objectlist, test_no_response_model_object).
Note that the tests also pass with a pre 0.67 version of FastApi (before official dataclass response support).

@zroger
Copy link

zroger commented Jun 28, 2022

I am having this same issue, and was going to propose the same fix.

I think test cases should be added to test_jsonable_encoder.py since the jsonable_encoder is the affected component, and it can be used for other purposes than just response serialization.

@codecov
Copy link

codecov bot commented Aug 26, 2022

Codecov Report

Merging #3607 (e722c7b) into master (26c68c6) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #3607   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          535       535           
  Lines        13828     13831    +3     
=========================================
+ Hits         13828     13831    +3     
Impacted Files Coverage Δ
fastapi/encoders.py 100.00% <100.00%> (ø)
tests/test_serialize_response_dataclass.py 100.00% <100.00%> (ø)

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

@github-actions
Copy link
Contributor

📝 Docs preview for commit e722c7b at: https://6308d0a41d7c7029546073b2--fastapi.netlify.app

@tiangolo tiangolo changed the title Fix jsonable_encoder for dataclasses with pydantic-compatible fields 🐛 Fix jsonable_encoder for dataclasses with pydantic-compatible fields Aug 26, 2022
@tiangolo
Copy link
Owner

Amazing, thanks @himbeles! And thanks for adding several tests! 🚀 🍰

@tiangolo tiangolo merged commit 22bed00 into tiangolo:master Aug 26, 2022
@himbeles himbeles deleted the fix-dataclass-jsonable-encoder branch August 26, 2022 15:48
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

3 participants