Skip to content

Conversation

sydney-runkle
Copy link
Contributor

@sydney-runkle sydney-runkle commented Jan 9, 2025

Following up on removing _std_types_schema.py and removing prepare annotations logic to increase consistency of annotation application patterns across the codebase.

Specifically, the third and last in the series of #10846 and #11239.

Seeing marginal perf improvements in GenerateSchema._prepare_annotations, which is what we were hoping for: https://codspeed.io/pydantic/pydantic/branches/final-removal-of-prep-ann

I've attempted to keep the structure of the core schema gen + validators close to that of the original. I think this could be refactored, but I'd rather do that in a different PR, and have this just focus on movement of the logic.

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Jan 9, 2025
@sydney-runkle sydney-runkle added relnotes-change Used for changes to existing functionality which don't have a better categorization. refactor modification of internal structures / patterns relnotes-performance Used for performance improvements. and removed relnotes-fix Used for bugfixes. labels Jan 9, 2025
Copy link

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

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2b3bd7b
Status: ✅  Deploy successful!
Preview URL: https://5ba55455.pydantic-docs.pages.dev
Branch Preview URL: https://final-removal-of-prep-ann.pydantic-docs.pages.dev

View logs

Copy link

codspeed-hq bot commented Jan 9, 2025

CodSpeed Performance Report

Merging #11247 will not alter performance

Comparing final-removal-of-prep-ann (2b3bd7b) with main (4e055d5)

Summary

✅ 46 untouched benchmarks

@sydney-runkle
Copy link
Contributor Author

I feel like test_nested_model_serialization might be flaky, this PR should be unrelated...

@sydney-runkle sydney-runkle changed the title Draft: move Mapping schema gen to GenerateSchema Move Mapping schema gen to GenerateSchema to complete removal of prepare_annotations_for_known_type workaround Jan 10, 2025
Copy link
Contributor

github-actions bot commented Jan 10, 2025

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic/_internal
  _generate_schema.py 1031
  _validators.py 457
Project Total  

This report was generated by python-coverage-comment-action

Comment on lines -1090 to -1094
res = self._get_prepare_pydantic_annotations_for_known_type(obj, ())
if res is not None:
source_type, annotations = res
return self._apply_annotations(source_type, annotations)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yay! This is what we've been trying to remove for a long time :)

@sydney-runkle sydney-runkle requested a review from Viicos January 10, 2025 17:09

# Assume Annotated[..., Field(...)]
if _typing_extra.is_annotated(values_source_type):
# Important that we use typing_extensions.get_args here in order to support 3.8
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Important that we use typing_extensions.get_args here in order to support 3.8

Or in the PR droping 3.8, as you wish

@sydney-runkle sydney-runkle merged commit aac7a53 into main Jan 10, 2025
60 checks passed
@sydney-runkle sydney-runkle deleted the final-removal-of-prep-ann branch January 10, 2025 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor modification of internal structures / patterns relnotes-change Used for changes to existing functionality which don't have a better categorization. relnotes-performance Used for performance improvements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants