Unpack PEP 695 type aliases if using the Annotated form
#11109
Merged
Conversation
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
Deploying pydantic-docs with
|
| Latest commit: |
848d3ee
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://011878fd.pydantic-docs.pages.dev |
| Branch Preview URL: | https://fields-pep695.pydantic-docs.pages.dev |
CodSpeed Performance ReportMerging #11109 will not alter performanceComparing Summary
|
a999599 to
7987e26
Compare
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||
aed47cc to
ec8f53b
Compare
Annotated form
Annotated formAnnotated form
ec8f53b to
da649cd
Compare
Annotated formAnnotated form during fields collection
1 task
Viicos
commented
Dec 17, 2024
pydantic/fields.py
Outdated
Comment on lines
405
to
407
| # 2. Check if the annotation is an `Annotated` form. | ||
| # In this case, `annotation` will be the annotated type: | ||
| annotation, metadata = _unpack_annotated(annotation) |
There was a problem hiding this comment.
This replaces:
if _typing_extra.is_annotated(annotation):
first_arg, *extra_args = typing_extensions.get_args(annotation)if metadata == [], it means _typing_extra.is_annotated(annotation) == False.
Viicos
commented
Dec 17, 2024
Viicos
commented
Dec 17, 2024
| @@ -314,28 +314,14 @@ def test_recursive_generic_type_alias_annotated_defs() -> None: | |||
| } | |||
|
|
|||
|
|
|||
| @pytest.mark.xfail(reason='description is currently dropped') | |||
| def test_field() -> None: | |||
There was a problem hiding this comment.
Now works, was moved below
Viicos
commented
Dec 17, 2024
Comment on lines
+1976
to
+1978
| # Ideally, we should delegate all this to `_typing_extra.unpack_annotated`, e.g.: | ||
| # `typ, annotations = _typing_extra.unpack_annotated(annotated_type); schema = self.apply_annotations(...)` | ||
| # if it was able to use a `NsResolver`. But because `unpack_annotated` is also used | ||
| # when constructing `FieldInfo` instances (where we don't have access to a `NsResolver`), | ||
| # the implementation of the function does *not* resolve forward annotations. This could | ||
| # be solved by calling `unpack_annotated` directly inside `collect_model_fields`. | ||
| # For now, we at least resolve the annotated type if it is a forward ref, but note that | ||
| # unexpected results will happen if you have something like `Annotated[Alias, ...]` and | ||
| # `Alias` is a PEP 695 type alias containing forward references. |
There was a problem hiding this comment.
See test_nested_annotated_with_type_aliases_and_forward_ref showing an example of when it fails.
There was a problem hiding this comment.
Yeesh. This comment is helpful, as this logic is pretty specialized and complicated. Thanks for including.
I honestly don't know if my mind is fully wrapped around this yet.
Annotated form during fields collectionAnnotated form
389f32a to
3153ed2
Compare
Comment on lines
+1976
to
+1978
| # Ideally, we should delegate all this to `_typing_extra.unpack_annotated`, e.g.: | ||
| # `typ, annotations = _typing_extra.unpack_annotated(annotated_type); schema = self.apply_annotations(...)` | ||
| # if it was able to use a `NsResolver`. But because `unpack_annotated` is also used | ||
| # when constructing `FieldInfo` instances (where we don't have access to a `NsResolver`), | ||
| # the implementation of the function does *not* resolve forward annotations. This could | ||
| # be solved by calling `unpack_annotated` directly inside `collect_model_fields`. | ||
| # For now, we at least resolve the annotated type if it is a forward ref, but note that | ||
| # unexpected results will happen if you have something like `Annotated[Alias, ...]` and | ||
| # `Alias` is a PEP 695 type alias containing forward references. |
There was a problem hiding this comment.
Yeesh. This comment is helpful, as this logic is pretty specialized and complicated. Thanks for including.
I honestly don't know if my mind is fully wrapped around this yet.
|
I feel ok approving this once you add new changes because:
|
1 task
29b3705 to
848d3ee
Compare
sydney-runkle
approved these changes
Dec 27, 2024
1 task
This was referenced Feb 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Change Summary
Fixes #6352
Fixes #10219
This PR also makes a first step in refactoring the
FieldInfoclass (see #11122), at least for thefrom_annotationconstructor method. The method logic is alsmost untouched, it was just adapted to use the new_unpack_annotatedutility function (also some variables had to be renamed) and comments were added to better describe the logic.The
from_annotated_attributeshould also be updated, but this is left for #11122.Related issue number
Checklist