Skip to content

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

Merged
merged 4 commits into from
Jan 14, 2025

Conversation

Viicos
Copy link
Member

@Viicos Viicos commented Jan 9, 2025

Change Summary

Fixes #11243.

Edit post 2.11 release: fixes #11133.

Best reviewed commit per commit.

Related issue number

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Copy link

cloudflare-workers-and-pages bot commented Jan 9, 2025

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: dbc6825
Status: ✅  Deploy successful!
Preview URL: https://425a87be.pydantic-docs.pages.dev
Branch Preview URL: https://dataclasses-fixes.pydantic-docs.pages.dev

View logs

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Jan 9, 2025
Copy link

codspeed-hq bot commented Jan 9, 2025

CodSpeed Performance Report

Merging #11246 will improve performances by 10.84%

Comparing dataclasses-fixes (dbc6825) with main (8ef33a0)

Summary

⚡ 1 improvements
✅ 44 untouched benchmarks

Benchmarks breakdown

Benchmark main dataclasses-fixes Change
test_construct_dataclass_schema 2.1 ms 1.9 ms +10.84%

@Viicos Viicos force-pushed the dataclasses-fixes branch from 3934346 to 40c7c32 Compare January 9, 2025 19:47
Copy link
Contributor

github-actions bot commented Jan 9, 2025

Coverage report

This PR does not seem to contain any modification to coverable code.

Copy link
Contributor

@sydney-runkle sydney-runkle left a 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!

Comment on lines +67 to +71
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]
Copy link
Contributor

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.
@Viicos Viicos force-pushed the dataclasses-fixes branch from 40c7c32 to 548348c Compare January 13, 2025 07:30
Copy link
Contributor

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Otherwise, lgtm!

@Viicos Viicos force-pushed the dataclasses-fixes branch from 548348c to dbc6825 Compare January 14, 2025 09:36
@Viicos Viicos enabled auto-merge (squash) January 14, 2025 09:39
@Viicos Viicos merged commit db061c2 into main Jan 14, 2025
58 checks passed
@Viicos Viicos deleted the dataclasses-fixes branch January 14, 2025 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes-fix Used for bugfixes.
Projects
None yet
2 participants