-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Respect config.alias_generator in pydantic dataclass #5673
Conversation
please review |
validation_alias: str | list[str | int] | list[list[str | int]] | None = common_field['validation_alias'] | ||
serialization_alias: str | None = common_field['serialization_alias'] | ||
alias_generator = self.config_wrapper.alias_generator | ||
if alias_generator: | ||
validation_alias = alias_generator(name) | ||
serialization_alias = alias_generator(name) | ||
|
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.
We can move this change into _common_field_schema but this function is used in _generate_td_field_schema
as well. So, that's why I keep it in the _generate_dc_field_schema
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 it may be a good idea to move into _common_field_schema
since soon there will be something equivalent for _generate_model_field_schema
(change coming down the pipe in pydantic core). Does it behave horribly/not work for typeddict if someone did add an alias? I think it's reasonable if someone might want to use camelCase aliases (specifically, via alias_generator) with typeddict too.
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 moved it to _common_field_schema
and also added a test for typeddict inspired by your example.
feca468
to
fc481e4
Compare
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.
Actually there is a problem here related to overriding aliases. I think we need to respect the alias priority stuff.
docs/usage/model_config.md
Outdated
@@ -263,7 +263,7 @@ print(Character.model_json_schema(by_alias=True)) | |||
{ | |||
'type': 'object', | |||
'properties': { | |||
'ActorName': {'type': 'string', 'default': None, 'title': 'Actorname'}, | |||
'Name': {'type': 'string', 'default': None, 'title': 'Name'}, |
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 don't think this attribute name should be changed, because the alias generator isn't supposed to override explicitly-set aliases.
The relevant attribute is FieldInfo.alias_priority
.
I think this shouldn't be too hard to fix, but we should look at how that is used now.
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 probably want to make a version of pydantic._internal._model_construction.apply_alias_generator
for dataclasses / typeddict
92a03d2
to
7be5b9e
Compare
please review |
7be5b9e
to
f8ec612
Compare
Fixes #4286
Selected Reviewer: @Kludex