Move Mapping schema gen to GenerateSchema to complete removal of prepare_annotations_for_known_type workaround#11247
Merged
sydney-runkle merged 6 commits intomainfrom Jan 10, 2025
Merged
Conversation
Deploying pydantic-docs with
|
| 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 |
CodSpeed Performance ReportMerging #11247 will not alter performanceComparing Summary
|
Contributor
Author
|
I feel like test_nested_model_serialization might be flaky, this PR should be unrelated... |
Mapping schema gen to GenerateSchemaMapping schema gen to GenerateSchema to complete removal of prepare_annotations_for_known_type workaround
Contributor
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||
sydney-runkle
commented
Jan 10, 2025
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) | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Yay! This is what we've been trying to remove for a long time :)
Viicos
approved these changes
Jan 10, 2025
|
|
||
| # 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 |
Member
There was a problem hiding this comment.
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
5 tasks
mberdyshev
reviewed
Apr 10, 2026
| return type_var_default_factory | ||
| elif values_type_origin not in allowed_default_types: | ||
| # a somewhat subjective set of types that have reasonable default values | ||
| allowed_msg = ', '.join([t.__name__ for t in set(allowed_default_types.values())]) |
There was a problem hiding this comment.
Shouldn't it be like this?
Suggested change
| allowed_msg = ', '.join([t.__name__ for t in set(allowed_default_types.values())]) | |
| allowed_msg = ', '.join([t.__name__ for t in set(allowed_default_types.keys())]) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following up on removing
_std_types_schema.pyand 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-annI'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.