This PR also makes a first step in refactoring theFieldInfo class (see#11122), at least for thefrom_annotation constructor method. The method logic is alsmost untouched, it was just adapted to use the new_unpack_annotated utility function (also some variables had to be renamed) and comments were added to better describe the logic.

Thefrom_annotated_attribute should also be updated, but this is left for#11122.

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

Eagerly evaluate type aliases if annotated

d0cb094

github-actionsbot added the relnotes-fixUsed for bugfixes. label

Dec 13, 2024

Copy link

cloudflare-workers-and-pagesbot commentedDec 13, 2024•
edited
Loading

Deploying pydantic-docs with Cloudflare Pages

Latest commit:	`848d3ee`
Status:	✅ Deploy successful!
Preview URL:	https://011878fd.pydantic-docs.pages.dev
Branch Preview URL:	https://fields-pep695.pydantic-docs.pages.dev

View logs

Copy link

codspeed-hqbot commentedDec 13, 2024•
edited
Loading

CodSpeed Performance Report

Merging#11109 willnot alter performance

_{Comparingfields-pep695 (848d3ee) withmain (aea47de)}

Summary

✅ 46 untouched benchmarks

Viicos force-pushed thefields-pep695 branch froma999599 to7987e26Compare

December 13, 2024 20:48

Copy link

Contributor

github-actionsbot commentedDec 13, 2024•
edited
Loading

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
pydantic
fields.py
pydantic/_internal
_generate_schema.py					1981,1983
_typing_extra.py					174-178,195-196
Project Total

_{This report was generated bypython-coverage-comment-action}

Viicos force-pushed thefields-pep695 branch 2 times, most recently fromaed47cc toec8f53bCompare

December 14, 2024 18:31

Viicos changed the title~~Eagerly evaluate type aliases if annotated~~Eagerly evaluate PEP 695type aliases if using theAnnotated form

Dec 14, 2024

Viicos changed the title~~Eagerly evaluate PEP 695type aliases if using theAnnotated form~~Eagerly evaluate PEP 695 type aliases if using theAnnotated form

Dec 14, 2024

Viicos marked this pull request as ready for review

December 14, 2024 18:36

Viicos force-pushed thefields-pep695 branch fromec8f53b toda649cdCompare

December 14, 2024 18:37

Viicos changed the title~~Eagerly evaluate PEP 695 type aliases if using theAnnotated form~~Unpack PEP 695 type aliases if using theAnnotated form during fields collection

Dec 14, 2024

Viicos mentioned this pull request

Dec 15, 2024

Field() andFieldInfo/metadata ordering inconsistencies and bugs#11122

Closed

1 task

Viicos commented

Dec 17, 2024

View reviewed changes

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)

Copy link

MemberAuthor

ViicosDec 17, 2024

Choose a reason for hiding this comment

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

This replaces:

if_typing_extra.is_annotated(annotation):first_arg,*extra_args=typing_extensions.get_args(annotation)

ifmetadata == [], it means_typing_extra.is_annotated(annotation) == False.

Viicos commented

Dec 17, 2024

View reviewed changes

pydantic/fields.pyShow resolvedHide resolved

Add tests, move function to right module, fix annotated schema

d663eed

Viicos commented

Dec 17, 2024

View reviewed changes

tests/test_type_alias_type.py

		@@ -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:

Copy link

MemberAuthor

ViicosDec 17, 2024

Choose a reason for hiding this comment

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

Now works, was moved below

Viicos commented

Dec 17, 2024

View reviewed changes

pydantic/_internal/_generate_schema.py

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.

Copy link

MemberAuthor

ViicosDec 17, 2024

Choose a reason for hiding this comment

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

Seetest_nested_annotated_with_type_aliases_and_forward_ref showing an example of when it fails.

Copy link

Contributor

sydney-runkleDec 20, 2024

Choose a reason for hiding this comment

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

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.

Fix fortyping-extensions@main

3153ed2

Viicos changed the title~~Unpack PEP 695 type aliases if using theAnnotated form during fields collection~~Unpack PEP 695 type aliases if using theAnnotated form

Dec 17, 2024

sydney-runkle added backport-2.10 and removed backport-2.10 labels

Dec 18, 2024

Viicos force-pushed thefields-pep695 branch from389f32a to3153ed2Compare

December 20, 2024 17:29

Viicos added the third-party-testsAdd this label on a PR to trigger 3rd party tests label

Dec 20, 2024

sydney-runkle reviewed

Dec 20, 2024

View reviewed changes

Copy link

Contributor

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.

I'm very impressed with your understanding of these nuances and edge cases here.

I've left a few questions / comments. Generally, looks good to me. Happy to do another quick pass after you've looked at feedback.

tests/test_type_alias_type.py OutdatedShow resolvedHide resolved

pydantic/fields.pyShow resolvedHide resolved

pydantic/_internal/_generate_schema.py

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.