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

Allow combining Field annotations by merging them #6398

Merged
merged 6 commits into from
Jul 3, 2023
Merged

Conversation

adriangb
Copy link
Member

@adriangb adriangb commented Jul 3, 2023

Fixes #6353, closes #6336

Selected Reviewer: @hramezani

@cloudflare-pages
Copy link

cloudflare-pages bot commented Jul 3, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: dbe22e8
Status: ✅  Deploy successful!
Preview URL: https://7169334a.pydantic-docs2.pages.dev
Branch Preview URL: https://merge-field.pydantic-docs2.pages.dev

View logs

@adriangb
Copy link
Member Author

adriangb commented Jul 3, 2023

@tiangolo I think this solves the problem you raised in #6336 (comment)

Comment on lines +1236 to +1237
if field_infos:
annotations = [*non_field_infos, FieldInfo.merge_field_infos(*field_infos)]
Copy link
Member Author

Choose a reason for hiding this comment

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

This re-ordering does have some possible consequences. In particular, each FieldInfo currently hooks into __get_pydantic_core_schema__ on its own so the order matters. But this re-ordering says "even if everything else runs in the order it is put into Annotated, we do special handling for Field() so that it runs at the end".

@adriangb
Copy link
Member Author

adriangb commented Jul 3, 2023

please review

@tiangolo
Copy link
Member

tiangolo commented Jul 3, 2023

Yesss! This solves it for me, and simplifies the code on FastAPI as I can now just use the TypeAdapter.core_schema directly. 🎉

If this goes in (and is released), I'll be able to make the final FastAPI beta, and then the final release in one or two days. 🚀

@adriangb adriangb merged commit 414f8ea into main Jul 3, 2023
50 checks passed
@adriangb adriangb deleted the merge-field branch July 3, 2023 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using multiple Fields in a single Annotated is broken
3 participants