Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Only evaluateFieldInfo annotations if required during schema building#10769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
sydney-runkle merged 2 commits intomainfromfieldinfo-evaluated
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from1 commit
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
NextNext commit
Only evaluateFieldInfo annotations if required during schema building
By adding a new `evaluated` attribute that can be set during modelfields collection. Also refactor a bit the utility functions to evalsuch annotations.
  • Loading branch information
@Viicos
Viicos committedNov 14, 2024
commit49c57bff949dbc556a2aa97639e18e2634909aa1
8 changes: 6 additions & 2 deletionspydantic/_internal/_fields.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -117,7 +117,7 @@ def collect_model_fields( # noqa: C901
fields: dict[str, FieldInfo] = {}

class_vars: set[str] = set()
for ann_name, ann_type in type_hints.items():
for ann_name,(ann_type, evaluated) in type_hints.items():
if ann_name == 'model_config':
# We never want to treat `model_config` as a field
# Note: we may need to change this logic if/when we introduce a `BareModel` class with no
Expand DownExpand Up@@ -202,6 +202,7 @@ def collect_model_fields( # noqa: C901
except AttributeError:
if ann_name in annotations:
field_info = FieldInfo_.from_annotation(ann_type)
field_info.evaluated = evaluated
else:
# if field has no default value and is not in __annotations__ this means that it is
# defined in a base class and we can take it from there
Expand All@@ -214,6 +215,8 @@ def collect_model_fields( # noqa: C901
# generated thanks to models not being fully defined while initializing recursive models.
# Nothing stops us from just creating a new FieldInfo for this type hint, so we do this.
field_info = FieldInfo_.from_annotation(ann_type)
field_info.evaluated = evaluated

else:
_warn_on_nested_alias_in_annotation(ann_type, ann_name)
if isinstance(default, FieldInfo_) and ismethoddescriptor(default.default):
Expand All@@ -224,6 +227,7 @@ def collect_model_fields( # noqa: C901
default.default = default.default.__get__(None, cls)

field_info = FieldInfo_.from_annotated_attribute(ann_type, default)
field_info.evaluated = evaluated
# attributes which are fields are removed from the class namespace:
# 1. To match the behaviour of annotation-only fields
# 2. To avoid false positives in the NameError check above
Expand DownExpand Up@@ -316,7 +320,7 @@ def collect_dataclass_fields(
continue

globalns, localns = ns_resolver.types_namespace
ann_type= _typing_extra.eval_type(dataclass_field.type, globalns, localns, lenient=True)
ann_type, _= _typing_extra.try_eval_type(dataclass_field.type, globalns, localns)

if _typing_extra.is_classvar_annotation(ann_type):
continue
Expand Down
19 changes: 12 additions & 7 deletionspydantic/_internal/_generate_schema.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1221,11 +1221,15 @@ def _common_field_schema( # C901
) -> _CommonField:
# Update FieldInfo annotation if appropriate:
FieldInfo = import_cached_field_info()
if has_instance_in_type(field_info.annotation, (ForwardRef, str)):
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

The interesting part is that we remove the call tohas_instance_in_type here.

sydney-runkle reacted with thumbs up emoji
# TODO Can we use field_info.apply_typevars_map here? Shouldn't we use lenient=False?
evaluated = _typing_extra.eval_type(field_info.annotation, *self._types_namespace, lenient=True)
if not field_info.evaluated:
# TODO Can we use field_info.apply_typevars_map here?
try:
evaluated = _typing_extra.eval_type(field_info.annotation, *self._types_namespace)
except NameError as e:
raise PydanticUndefinedAnnotation.from_name_error(e) from e
evaluated = replace_types(evaluated, self._typevars_map)
if evaluated is not field_info.annotation and not has_instance_in_type(evaluated, PydanticRecursiveRef):
field_info.evaluated = True
if not has_instance_in_type(evaluated, PydanticRecursiveRef):
new_field_info = FieldInfo.from_annotation(evaluated)
field_info.annotation = new_field_info.annotation

Expand DownExpand Up@@ -1344,12 +1348,13 @@ def _type_alias_type_schema(self, obj: TypeAliasType) -> CoreSchema:
return maybe_schema

origin: TypeAliasType = get_origin(obj) or obj

annotation = origin.__value__
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Moved below, as accessing the__value__ attribute will trigger the evaluation of the annotation, and thus this can raise aNameError. Theeval_type call is still necessary though, as you can have explicit string references inside a a type alias type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I'm misunderstanding - was this moved below? Or do we not need this specific line anymore?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Basically, here is what can happen with type aliases:

typeA=Intannotation=origin.__value__# NameError, 'Int' is not definedtypeB='Int'annotation=origin.__value__assertannotation=='Int'eval_type(B, ..., ...)# NameError, 'Int' is not defined

So everything was moved in a singletry..except block:

try:annotation=_typing_extra.eval_type(origin.__value__,*self._types_namespace)except ...: ...

sydney-runkle reacted with thumbs up emoji
typevars_map = get_standard_typevars_map(obj)

with self._ns_resolver.push(origin):
annotation = _typing_extra.eval_type(annotation, *self._types_namespace, lenient=True)
try:
annotation = _typing_extra.eval_type(origin.__value__, *self._types_namespace)
except NameError as e:
raise PydanticUndefinedAnnotation.from_name_error(e) from e
annotation = replace_types(annotation, typevars_map)
schema = self.generate_schema(annotation)
assert schema['type'] != 'definitions'
Expand Down
99 changes: 76 additions & 23 deletionspydantic/_internal/_typing_extra.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,7 +9,7 @@
import typing
import warnings
from functools import lru_cache, partial
from typing import Any, Callable
from typing import Any, Callable, Literal, overload

import typing_extensions
from typing_extensions import TypeIs, deprecated, get_args, get_origin
Expand DownExpand Up@@ -449,17 +449,54 @@ def parent_frame_namespace(*, parent_depth: int = 2, force: bool = False) -> dic
return frame.f_locals


def _type_convert(arg: Any) -> Any:
"""Convert `None` to `NoneType` and strings to `ForwardRef` instances.

This is a backport of the private `typing._type_convert` function. When
evaluating a type, `ForwardRef._evaluate` ends up being called, and is
responsible for making this conversion. However, we still have to apply
it for the first argument passed to our type evaluation functions, similarly
to the `typing.get_type_hints` function.
"""
if arg is None:
return NoneType
if isinstance(arg, str):
# Like `typing.get_type_hints`, assume the arg can be in any context,
# hence the proper `is_argument` and `is_class` args:
return _make_forward_ref(arg, is_argument=False, is_class=True)
return arg


@overload
def get_cls_type_hints(
obj: type[Any], *, ns_resolver: NsResolver | None = None, lenient: bool = False
) -> dict[str, Any]:
obj: type[Any],
*,
ns_resolver: NsResolver | None = None,
lenient: Literal[True],
) -> dict[str, tuple[Any, bool]]: ...
@overload
def get_cls_type_hints(
obj: type[Any],
*,
ns_resolver: NsResolver | None = None,
lenient: Literal[False] = ...,
) -> dict[str, Any]: ...
def get_cls_type_hints(
obj: type[Any],
*,
ns_resolver: NsResolver | None = None,
lenient: bool = False,
) -> dict[str, Any] | dict[str, tuple[Any, bool]]:
"""Collect annotations from a class, including those from parent classes.

Args:
obj: The class to inspect.
ns_resolver: A namespace resolver instance to use. Defaults to an empty instance.
lenient: Whether to keep unresolvable annotations as is or re-raise the `NameError` exception. Default: re-raise.
lenient: Whether to keep unresolvable annotations as is or re-raise the `NameError` exception.
If lenient, an extra boolean flag is set for each annotation value to indicate whether the
evaluation succeeded or not. Default: re-raise.
"""
hints = {}
hints: dict[str, Any] | dict[str, tuple[Any, bool]] = {}
ns_resolver = ns_resolver or NsResolver()

for base in reversed(obj.__mro__):
Expand All@@ -469,50 +506,66 @@ def get_cls_type_hints(
with ns_resolver.push(base):
globalns, localns = ns_resolver.types_namespace
for name, value in ann.items():
hints[name] = eval_type(value, globalns, localns, lenient=lenient)
if lenient:
hints[name] = try_eval_type(value, globalns, localns)
else:
hints[name] = eval_type(value, globalns, localns)
return hints


defeval_type(
deftry_eval_type(
value: Any,
globalns: GlobalsNamespace | None = None,
localns: MappingNamespace | None = None,
*,
lenient: bool = False,
) -> Any:
"""Evaluate the annotation using the provided namespaces.
) -> tuple[Any, bool]:
"""Try evaluating the annotation using the provided namespaces.

Args:
value: The value to evaluate. If `None`, it will be replaced by `type[None]`. If an instance
of `str`, it will be converted to a `ForwardRef`.
localns: The global namespace to use during annotation evaluation.
globalns: The local namespace to use during annotation evaluation.
lenient: Whether to keep unresolvable annotations as is or re-raise the `NameError` exception. Default: re-raise.

Returns:
A two-tuple containing the possibly evaluated type and a boolean indicating
whether the evaluation succeeded or not.
"""
if value is None:
value = NoneType
elif isinstance(value, str):
value = _make_forward_ref(value, is_argument=False, is_class=True)
value = _type_convert(value)

try:
return eval_type_backport(value, globalns, localns)
return eval_type_backport(value, globalns, localns), True
except NameError:
if not lenient:
raise
# the point of this function is to be tolerant to this case
return value
return value, False


def eval_type(
value: Any,
globalns: GlobalsNamespace | None = None,
localns: MappingNamespace | None = None,
) -> Any:
"""Evaluate the annotation using the provided namespaces.

Args:
value: The value to evaluate. If `None`, it will be replaced by `type[None]`. If an instance
of `str`, it will be converted to a `ForwardRef`.
localns: The global namespace to use during annotation evaluation.
globalns: The local namespace to use during annotation evaluation.
"""
value = _type_convert(value)
return eval_type_backport(value, globalns, localns)


@deprecated(
'`eval_type_lenient` is deprecated, use `eval_type` with `lenient=True` instead.',
'`eval_type_lenient` is deprecated, use `try_eval_type` instead.',
Comment on lines -507 to +559
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this feels like an anti-pattern - if we already deprecated this, there's probably a reason why we didn't want to have two functions. Even if we disagree with this deprecation now, seems like a lot of churn to basically reintroduce the function and redirect for the deprecated case...

I believe my suggestion above re always returning a second value,evaluated, even in the case ofeval_type withlenient=False might help, and we can leave this be?

Copy link
Contributor

Choose a reason for hiding this comment

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

A few more follow ups:

  1. This is a very internal function - can we just deprecate this?
  2. If we were to bring back a second function, to reduce churn, can we call iteval_type_lenient?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I had to keep this function during the types namespaces refactor because fastapi makes use of it 🫤. I added thedeprecated decorator so that type checking fails in the fastapi CI and they can use the correct function (or best, do not rely on our internals because I don't think the_typing_extra module is stable enough, we might apply other changes here in the future).

I believe my suggestion above re always returning a second value,evaluated, even in the case ofeval_type withlenient=False might help, and we can leave this be?

Same issue ashttps://github.com/pydantic/pydantic/pull/10769/files#r1832947088, we can't have a single function because we want to catch theNameError to reraisePydanticUndefinedAnnotation

sydney-runkle reacted with thumbs up emoji
category=None,
)
def eval_type_lenient(
value: Any,
globalns: GlobalsNamespace | None = None,
localns: MappingNamespace | None = None,
) -> Any:
return eval_type(value, globalns, localns, lenient=True)
ev, _ = try_eval_type(value, globalns, localns)
return ev


def eval_type_backport(
Expand Down
10 changes: 6 additions & 4 deletionspydantic/fields.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -154,6 +154,7 @@ class FieldInfo(_repr.Representation):

__slots__ = (
'annotation',
'evaluated',
'default',
'default_factory',
'alias',
Expand DownExpand Up@@ -207,6 +208,7 @@ def __init__(self, **kwargs: Unpack[_FieldInfoInputs]) -> None:
self._attributes_set = {k: v for k, v in kwargs.items() if v is not _Unset}
kwargs = {k: _DefaultValues.get(k) if v is _Unset else v for k, v in kwargs.items()} # type: ignore
self.annotation, annotation_metadata = self._extract_metadata(kwargs.get('annotation'))
self.evaluated = False

default = kwargs.pop('default', PydanticUndefined)
if default is Ellipsis:
Expand DownExpand Up@@ -650,17 +652,17 @@ def apply_typevars_map(
pydantic._internal._generics.replace_types is used for replacing the typevars with
their concrete types.
"""
annotation= _typing_extra.eval_type(self.annotation, globalns, localns, lenient=True)
annotation, _= _typing_extra.try_eval_type(self.annotation, globalns, localns)
self.annotation = _generics.replace_types(annotation, typevars_map)

def __repr_args__(self) -> ReprArgs:
yield 'annotation', _repr.PlainRepr(_repr.display_as_type(self.annotation))
yield 'required', self.is_required()

for s in self.__slots__:
if s == '_attributes_set':
continue
if s== 'annotation':
# TODO: properly make use of the protocol (https://rich.readthedocs.io/en/stable/pretty.html#rich-repr-protocol)
# By yielding a three-tuple:
if sin ('_attributes_set', 'annotation', 'evaluated'):
continue
elif s == 'metadata' and not self.metadata:
continue
Expand Down
[8]ページ先頭
©2009-2025 Movatter.jp