-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Fix various issues with dataclasses and use_attribute_docstrings
#11246
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
Deploying pydantic-docs with
|
Latest commit: |
dbc6825
|
Status: | ✅ Deploy successful! |
Preview URL: | https://425a87be.pydantic-docs.pages.dev |
Branch Preview URL: | https://dataclasses-fixes.pydantic-docs.pages.dev |
CodSpeed Performance ReportMerging #11246 will improve performances by 10.84%Comparing Summary
Benchmarks breakdown
|
3934346
to
40c7c32
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.
Nice work overall, left a few comments / questions. Thanks!
def _update_fields_from_docstrings(cls: type[Any], fields: dict[str, FieldInfo], use_inspect: bool = False) -> None: | ||
fields_docs = extract_docstrings_from_cls(cls, use_inspect=use_inspect) | ||
for ann_name, field_info in fields.items(): | ||
if field_info.description is None and ann_name in fields_docs: | ||
field_info.description = fields_docs[ann_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.
Re letting callers decide if this is necessary - I assume this is for general cleanliness and to avoid the overhead of a function call if not necessary?
…in the process of being built Do not rely on `__pydantic_validator__`, an attribute which is set only when the class is fully built. Because we check for this condition during schema building, we end up considering the class as a stdlib dataclass. As a consequence, the deepcopy logic of `FieldInfo` instances was tweaked, as in some tests, the deep copy would fail. Instead, make a shallow copy of every `FieldInfo` instance, this is enough (the same thing is done in `collect_model_fields` for Pydantic models, we make a shallow copy of the parent fields).
Let callers be responsible for checking if using docstrings is necessary.
40c7c32
to
548348c
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.
Otherwise, lgtm!
548348c
to
dbc6825
Compare
Change Summary
Fixes #11243.
Edit post 2.11 release: fixes #11133.
Best reviewed commit per commit.
Related issue number
Checklist