-
-
Notifications
You must be signed in to change notification settings - Fork 421
Fix pydantic_model_creator
incompatibility with Pydantic 2.11 (#1925)
#1930
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
base_model = type( | ||
"BasePydanticModel", | ||
(PydanticModel,), | ||
{"model_config": self._pconfig, **computed_fields}, |
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.
Note that you can specify config in create_model()
via the __config__
parameter.
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.
Thanks for the review. I did notice this parameter while checking the source code, but it seems __config__
cannot be used together with __base__
.
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.
Hum, this limitation was introduced a really long time ago, when create_model()
was added in pydantic/pydantic#125. I'm pretty sure this limitation can be removed now; it is possible to specify model_config
on a subclass inheriting from (multiple) bases. This will be tracked in pydantic/pydantic#2137.
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.
@Viicos pydantic I assume not accepting model_config as field parameter to create_model is a regression introduced in 2.11.0?
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.
If it was working before 2.11, this was by accident. The proper and documented way was (and still is) to provide configuration using the __config__
parameter.
CodSpeed Performance ReportMerging #1930 will not alter performanceComparing Summary
|
Pull Request Test Coverage Report for Build 14303714936Details
💛 - Coveralls |
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.
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
Files not reviewed (1)
- CHANGELOG.rst: Language not supported
Comments suppressed due to low confidence (2)
tortoise/contrib/pydantic/creator.py:391
- [nitpick] Consider using hasattr(v, 'decorator_info') to check for the presence of a computed field attribute, which may more clearly express your intent.
if isinstance(getattr(v, "decorator_info", None), ComputedFieldInfo):
tortoise/contrib/pydantic/creator.py:402
- Verify that all necessary properties from self._properties are preserved when splitting into common_fields and computed_fields. Ensure that the base_model fully replicates the configuration required by downstream usage.
__base__=base_model,
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.
The change looks good to me, thank you for your contribution!
Hi, I'm with you. |
Description
Modified the parameters passed to Pydantic's
create_model
method to ensure compatibility with Pydantic 2.11.Motivation and Context
Fixes #1925
How Has This Been Tested?
make test
Checklist: