-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
⚡️ Fix default_factory for response model field with Pydantic V1
#9704
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 default_factory for response model field with Pydantic V1
#9704
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
📝 Docs preview for commit c575414 at: https://649474bddaa2444d0ae2bad3--fastapi.netlify.app |
|
@tiangolo I just resolved the conflict, can you take a look at it? Any plans to merge? |
|
Hi @vvanglro, thank you for being so interested in contributing to FastAPI. We appreciate your effort and the time you have taken to submit this PR. We have a high volume of PRs and we're working hard to review and classify them. We'll review your PR soon 🙇♀️ |
YuriiMotov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This issue is only relevant when working with Pydantic V1.
I simplified tests a bit.
@vvanglro, thanks for working on this!
default_factory for response model field with Pydantic V1
|
@YuriiMotov Everything seems to be in order. Let's merge. |
tiangolo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thank you! 🍰
And thanks a lot @YuriiMotov for the work on this! ☕
@tiangolo Hi, When returning a dictionary, if there is a
default_factoryin the model, it will not take effect, and the return value is None. Normal if returned as a model.The
default_factoryadded by pydantic in version 1.5, I checked the documentation and it seems that it is still in the beta stage, so is this the reason does not supportdefault_factoryor is there another reason? But I think the consistency of the two return methods should be kept.Related Documentation Links:
https://docs.pydantic.dev/latest/usage/models/#field-with-dynamic-default-value
pydantic/pydantic#866
Related Issues:
#8394
#8853
Here is a MRE:
UPD YuriiMotov:
original code example in details
Details