-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Fix usage of AliasGenerator with computed_field decorator
#8806
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
Conversation
…dantic into fix-alex-suggestions
Deploying with
|
| Latest commit: |
ef959d9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://82e9754c.pydantic-docs2.pages.dev |
| Branch Preview URL: | https://alias-gen-fixes.pydantic-docs2.pages.dev |
|
Please review |
CodSpeed Performance ReportMerging #8806 will not alter performanceComparing Summary
|
| # note that we use the serialization_alias with priority over alias, as computed_field | ||
| # aliases are used for serialization only (not validation) | ||
| if computed_field_info.alias_priority == 1: | ||
| computed_field_info.alias = serialization_alias or alias |
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.
I would do
| computed_field_info.alias = serialization_alias or alias | |
| computed_field_info.alias = serialization_alias if serialization_alias is not None else alias |
to handle the case of serialization_alias = '' and alias != ''. Probably worth a comment about why, though not a test.
I could be convinced not to do that but I do think there are tests for the alias = '' case elsewhere in the codebase.
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.
I think we should do this, but in a different PR, to both the field_info logic and the computed_field_info logic.
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.
Also, doesn't seem like an urgent change 👍
Co-authored-by: Alex Hall <alex.mojaki@gmail.com>
Fix #8768
Ensure that
AliasGeneratoris handled properly when using it along with acomputed_fieldin a model.Selected Reviewer: @samuelcolvin