Skip to content

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