Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.2k
Addrebuild() method forTypeAdapter and simplifydefer_build patterns#10537
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
cloudflare-workers-and-pagesbot commentedOct 1, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Deploying pydantic-docs with |
| Latest commit: | b14206b |
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b431536a.pydantic-docs.pages.dev |
| Branch Preview URL: | https://ta-defer-build-simplificatio.pydantic-docs.pages.dev |
codspeed-hqbot commentedOct 1, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
CodSpeed Performance ReportMerging#10537 willimprove performances by 17.17%Comparing Summary
Benchmarks breakdown
|
rebuild() method forTypeAdapter and simplifydefer_build patternsMy best guess regarding why the deferred parent namespace fetching was implemented was to support the following: frompydanticimportTypeAdapterdeffunc():ta=TypeAdapter("Forward",config={"defer_build":True})Forward=intta.core_schema# triggers the core schema build, and `frame.f_locals` is only fetched at this point, so it includes `Forward` Which presumably won't work anymore with this PR. I'll not that this pattern feels "dangerous", as one can do: deffunc():ta=TypeAdapter("Forward",config={"defer_build":True})Forward=intreturntata=func()ta.core_schema# PydanticUndefinedAnnotation: name 'Forward' is not defined, because locals were gc'ed.# Things could also be worse, if you defined `Forward = str` before calling `ta.core_schema` here So I think we need to be careful here before making the change. People might be relying on this behavior? |
I'd argue that we shouldnt' support rebuilds on attribute accesses for Indeed, this is an issue, but it's one that we run into with |
I'll note, this works: frompydanticimportBaseModel,TypeAdapter,ConfigDictInt=intdeftester_model()->None:classModel(BaseModel):i:Intmodel_config=ConfigDict(defer_build=True)print(Model.__pydantic_core_schema__)# mock core schemaModel.model_rebuild()print(Model.__pydantic_core_schema__)# works, displays a core schematester_model()deftester_adapter()->None:ta=TypeAdapter(Int,config=ConfigDict(defer_build=True))print(ta.core_schema)# mock core schemata.rebuild()print(ta.core_schema)# works, displays a core schematester_adapter() |
Admittedly, I don't think this should undergo serious review until we merge#10530 |
Closing based on#10632. Will resume in v2.11 |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| type_repr: Name of the type used in the adapter, used in error messages | ||
| """ | ||
| undefined_type_error_message = ( | ||
| f'`TypeAdapter[{type_repr}]` is not fully defined; you should define `{type_repr}` and all referenced types,' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
TheTypeAdapter[...] form seems a bit weird, especially becausetype_repr seems to bestr(type) when called fromTypeAdapter. Probably fine as is, just wanted to note that this may lead to weird string representations (maybe_display.display_as_type could be used, although it is costly to compute).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
you could probably rework the APIs a bit to make it possible to defer the cost of computing the error message if it's a concern. Like, make MockCoreSchema accept a callable for the error message rather than just a string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Suggested the same in othercomment about making the error message forming lazy. So it only happens on errors
Uh oh!
There was an error while loading.Please reload this page.
sydney-runkle commentedNov 12, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
EDIT: reducing this to closed within a note now that this has been resolved DetailsSo I'll note, this works: fromtypingimportGeneric,Optional,TypeVarfromtyping_extensionsimportNamedTupleasTypingExtensionsNamedTuplefrompydanticimportTypeAdapterT=TypeVar('T')classMyNamedTuple(TypingExtensionsNamedTuple,Generic[T]):x:Ty:Optional['MyNamedTuple[T]']ta=TypeAdapter(MyNamedTuple[int])print(ta.validate_python((1,None)))#> MyNamedTuple(x=1, y=None) So does this (with the same imports as above) T=TypeVar('T')deftest_in_function ()->None:classMyNamedTuple(TypingExtensionsNamedTuple,Generic[T]):x:Ty:Optional['MyNamedTuple[T]']ta=TypeAdapter(MyNamedTuple[int])print(ta.validate_python((1,None)))test_in_function()#> MyNamedTuple(x=1, y=None) But alas, this, does not. Debugging now. deftest_in_function ()->None:T=TypeVar('T')classMyNamedTuple(TypingExtensionsNamedTuple,Generic[T]):x:Ty:Optional['MyNamedTuple[T]']ta=TypeAdapter(MyNamedTuple[int])print(ta.validate_python((1,None)))test_in_function() |
| _parent_depth: Depth at which to search the for the parent namespace used for schema building. | ||
| We also use this as a reference level to find the global namespace (_parent_depth - 1). | ||
| Defaults to 2 because we expect to reference the frame that called the `TypeAdapter` constructor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think it's worth considering - should we default to1, then add1 when we call intoparent_frame_namespace? That seems more intuitive to me from a user perspective. cc@Viicos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Though I think this could be a valuable change, it is breaking, as folks are depending on the current behavior. This is_ prefixed though, so we sort of have range to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It could make more sense but would be inconsistent withmodel_rebuild which suffers from the same issue :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Alrighty, let's leave as is :(
cc@MarkusSintonen, if you're available, this should be ready for another review :) |
Uh oh!
There was an error while loading.Please reload this page.
| schema_generator = _generate_schema.GenerateSchema(config_wrapper, ns_resolver=ns_resolver) | ||
| try: | ||
| core_schema = schema_generator.generate_schema(self._type) | ||
| except PydanticUndefinedAnnotation: | ||
| if raise_errors: | ||
| raise | ||
| _mock_val_ser.set_type_adapter_mocks(self, str(self._type)) | ||
| return False | ||
| try: | ||
| self.core_schema = schema_generator.clean_schema(core_schema) | ||
| except schema_generator.CollectedInvalid: | ||
| _mock_val_ser.set_type_adapter_mocks(self, str(self._type)) | ||
| return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I have seen the same try-catch-if-raise_error-mocking logic repeated in many places. Wondering could it be extracted to some common handling... Maybe later...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, let's consolidate later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Maybe with341a778, I can create an issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
sydney-runkle commentedNov 13, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Another idea I had, which doesn't fit in scope here. See:341a778 Detailsdiff --git a/pydantic/_internal/_mock_val_ser.py b/pydantic/_internal/_mock_val_ser.pyindex ddf0a6ac7..9bd8be7d4 100644--- a/pydantic/_internal/_mock_val_ser.py+++ b/pydantic/_internal/_mock_val_ser.py@@ -1,6 +1,7 @@ from __future__ import annotations-from typing import TYPE_CHECKING, Any, Callable, Generic, Iterator, Mapping, TypeVar, Union+from functools import partial+from typing import TYPE_CHECKING, Any, Callable, Generic, Iterator, Mapping, TypeAlias, TypeVar, Union from pydantic_core import CoreSchema, SchemaSerializer, SchemaValidator from typing_extensions import Literal@@ -13,9 +14,8 @@ if TYPE_CHECKING: from ..main import BaseModel from ..type_adapter import TypeAdapter--ValSer = TypeVar('ValSer', bound=Union[SchemaValidator, PluggableSchemaValidator, SchemaSerializer])-T = TypeVar('T')+ValidatorOrSerializer: TypeAlias = Union[SchemaValidator, PluggableSchemaValidator, SchemaSerializer]+ValSer = TypeVar('ValSer', bound=ValidatorOrSerializer) class MockCoreSchema(Mapping[str, Any]):@@ -109,44 +109,80 @@ class MockValSer(Generic[ValSer]): return None-def set_type_adapter_mocks(adapter: TypeAdapter, type_repr: str) -> None:- """Set `core_schema`, `validator` and `serializer` to mock core types on a type adapter instance.+MockContainer = TypeVar('MockContainer', bound=type[BaseModel] | TypeAdapter | type[PydanticDataclass])+RebuildReturnType = TypeVar('RebuildReturnType', bound=CoreSchema | ValidatorOrSerializer)- Args:- adapter: The type adapter instance to set the mocks on- type_repr: Name of the type used in the adapter, used in error messages- """- undefined_type_error_message = (- f'`TypeAdapter[{type_repr}]` is not fully defined; you should define `{type_repr}` and all referenced types,'- f' then call `.rebuild()` on the instance.'- )- def attempt_rebuild_fn(attr_fn: Callable[[TypeAdapter], T]) -> Callable[[], T | None]:- def handler() -> T | None:- if adapter.rebuild(_parent_namespace_depth=5) is not False:- return attr_fn(adapter)+class MockFactory(Generic[MockContainer]):+ """Factory for creating `MockCoreSchema`, `MockValSer` and `MockValSer` instances for a given type."""++ __slots__ = '_obj', '_error_message', '_rebuild'++ def __init__(+ self,+ obj: MockContainer,+ error_message: str,+ rebuild: Callable[[MockContainer], Callable[..., bool | None]],+ ) -> None:+ self._obj = obj+ self._error_message = error_message+ self._rebuild = rebuild++ def _attempt_rebuild_fn(+ self, attr_fn: Callable[[MockContainer], RebuildReturnType | None]+ ) -> Callable[[], RebuildReturnType | None]:+ def handler() -> RebuildReturnType | None:+ if self._rebuild(self._obj)(_parent_namespace_depth=5) is not False:+ return attr_fn(self._obj) else: return None return handler- adapter.core_schema = MockCoreSchema( # pyright: ignore[reportAttributeAccessIssue]- undefined_type_error_message,- code='class-not-fully-defined',- attempt_rebuild=attempt_rebuild_fn(lambda ta: ta.core_schema),- )- adapter.validator = MockValSer( # pyright: ignore[reportAttributeAccessIssue]- undefined_type_error_message,- code='class-not-fully-defined',- val_or_ser='validator',- attempt_rebuild=attempt_rebuild_fn(lambda ta: ta.validator),- )- adapter.serializer = MockValSer( # pyright: ignore[reportAttributeAccessIssue]- undefined_type_error_message,- code='class-not-fully-defined',- val_or_ser='serializer',- attempt_rebuild=attempt_rebuild_fn(lambda ta: ta.serializer),+ def mock_core_schema(self, attr_fn: Callable[[MockContainer], CoreSchema | None]) -> MockCoreSchema:+ return MockCoreSchema(+ self._error_message,+ code='class-not-fully-defined',+ attempt_rebuild=self._attempt_rebuild_fn(attr_fn),+ )++ def mock_schema_validator(+ self, attr_fn: Callable[[MockContainer], SchemaValidator | PluggableSchemaValidator | None]+ ) -> MockValSer:+ return MockValSer(+ self._error_message,+ code='class-not-fully-defined',+ val_or_ser='validator',+ attempt_rebuild=self._attempt_rebuild_fn(attr_fn),+ )++ def mock_schema_serializer(self, attr_fn: Callable[[MockContainer], SchemaSerializer | None]) -> MockValSer:+ return MockValSer(+ self._error_message,+ code='class-not-fully-defined',+ val_or_ser='serializer',+ attempt_rebuild=self._attempt_rebuild_fn(attr_fn),+ )+++def set_type_adapter_mocks(adapter: TypeAdapter, type_repr: str) -> None:+ """Set `core_schema`, `validator` and `serializer` to mock core types on a type adapter instance.++ Args:+ adapter: The type adapter instance to set the mocks on+ type_repr: Name of the type used in the adapter, used in error messages+ """+ mock_factory = MockFactory[TypeAdapter](+ obj=adapter,+ error_message=(+ f'`TypeAdapter[{type_repr}]` is not fully defined; you should define `{type_repr}` and all referenced types,'+ f' then call `.rebuild()` on the instance.'+ ),+ rebuild=lambda ta: partial(ta.rebuild, raise_errors=False), )+ adapter.core_schema = mock_factory.mock_core_schema(attr_fn=lambda ta: ta.core_schema) # pyright: ignore[reportAttributeAccessIssue]+ adapter.validator = mock_factory.mock_schema_validator(attr_fn=lambda ta: ta.validator) # pyright: ignore[reportAttributeAccessIssue]+ adapter.serializer = mock_factory.mock_schema_serializer(attr_fn=lambda ta: ta.serializer) # pyright: ignore[reportAttributeAccessIssue] def set_model_mocks(cls: type[BaseModel], cls_name: str, undefined_name: str = 'all referenced types') -> None:@@ -157,37 +193,18 @@ def set_model_mocks(cls: type[BaseModel], cls_name: str, undefined_name: str = ' cls_name: Name of the model class, used in error messages undefined_name: Name of the undefined thing, used in error messages """- undefined_type_error_message = (- f'`{cls_name}` is not fully defined; you should define {undefined_name},'- f' then call `{cls_name}.model_rebuild()`.'+ mock_factory = MockFactory[type[BaseModel]](+ obj=cls,+ error_message=(+ f'`{cls_name}` is not fully defined; you should define {undefined_name},'+ f' then call `{cls_name}.model_rebuild()`.'+ ),+ rebuild=lambda c: partial(c.model_rebuild, raise_errors=False), )- def attempt_rebuild_fn(attr_fn: Callable[[type[BaseModel]], T]) -> Callable[[], T | None]:- def handler() -> T | None:- if cls.model_rebuild(raise_errors=False, _parent_namespace_depth=5) is not False:- return attr_fn(cls)- else:- return None-- return handler-- cls.__pydantic_core_schema__ = MockCoreSchema( # pyright: ignore[reportAttributeAccessIssue]- undefined_type_error_message,- code='class-not-fully-defined',- attempt_rebuild=attempt_rebuild_fn(lambda c: c.__pydantic_core_schema__),- )- cls.__pydantic_validator__ = MockValSer( # pyright: ignore[reportAttributeAccessIssue]- undefined_type_error_message,- code='class-not-fully-defined',- val_or_ser='validator',- attempt_rebuild=attempt_rebuild_fn(lambda c: c.__pydantic_validator__),- )- cls.__pydantic_serializer__ = MockValSer( # pyright: ignore[reportAttributeAccessIssue]- undefined_type_error_message,- code='class-not-fully-defined',- val_or_ser='serializer',- attempt_rebuild=attempt_rebuild_fn(lambda c: c.__pydantic_serializer__),- )+ cls.__pydantic_core_schema__ = mock_factory.mock_core_schema(attr_fn=lambda c: c.__pydantic_core_schema__) # pyright: ignore[reportAttributeAccessIssue]+ cls.__pydantic_validator__ = mock_factory.mock_schema_validator(attr_fn=lambda c: c.__pydantic_validator__) # pyright: ignore[reportAttributeAccessIssue]+ cls.__pydantic_serializer__ = mock_factory.mock_schema_serializer(attr_fn=lambda c: c.__pydantic_serializer__) # pyright: ignore[reportAttributeAccessIssue] def set_dataclass_mocks(@@ -202,34 +219,15 @@ def set_dataclass_mocks( """ from ..dataclasses import rebuild_dataclass- undefined_type_error_message = (- f'`{cls_name}` is not fully defined; you should define {undefined_name},'- f' then call `pydantic.dataclasses.rebuild_dataclass({cls_name})`.'+ mock_factory = MockFactory[type[PydanticDataclass]](+ obj=cls,+ error_message=(+ f'`{cls_name}` is not fully defined; you should define {undefined_name},'+ f' then call `pydantic.dataclasses.rebuild_dataclass({cls_name})`.'+ ),+ rebuild=lambda c: partial(rebuild_dataclass, cls=c, raise_errors=False), )- def attempt_rebuild_fn(attr_fn: Callable[[type[PydanticDataclass]], T]) -> Callable[[], T | None]:- def handler() -> T | None:- if rebuild_dataclass(cls, raise_errors=False, _parent_namespace_depth=5) is not False:- return attr_fn(cls)- else:- return None-- return handler-- cls.__pydantic_core_schema__ = MockCoreSchema( # pyright: ignore[reportAttributeAccessIssue]- undefined_type_error_message,- code='class-not-fully-defined',- attempt_rebuild=attempt_rebuild_fn(lambda c: c.__pydantic_core_schema__),- )- cls.__pydantic_validator__ = MockValSer( # pyright: ignore[reportAttributeAccessIssue]- undefined_type_error_message,- code='class-not-fully-defined',- val_or_ser='validator',- attempt_rebuild=attempt_rebuild_fn(lambda c: c.__pydantic_validator__),- )- cls.__pydantic_serializer__ = MockValSer( # pyright: ignore[reportAttributeAccessIssue]- undefined_type_error_message,- code='class-not-fully-defined',- val_or_ser='serializer',- attempt_rebuild=attempt_rebuild_fn(lambda c: c.__pydantic_serializer__),- )+ cls.__pydantic_core_schema__ = mock_factory.mock_core_schema(attr_fn=lambda c: c.__pydantic_core_schema__) # pyright: ignore[reportAttributeAccessIssue]+ cls.__pydantic_validator__ = mock_factory.mock_schema_validator(attr_fn=lambda c: c.__pydantic_validator__) # pyright: ignore[reportAttributeAccessIssue]+ cls.__pydantic_serializer__ = mock_factory.mock_schema_serializer(attr_fn=lambda c: c.__pydantic_serializer__) # pyright: ignore[reportAttributeAccessIssue] @Viicos, you might appreciate this, seems better from a type checking perspective perhaps? Or just more consolidated generally. |
This reverts commit341a778.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We will still be able to tweak things before the final release if this break existing code with the next beta
c2647ab intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This started as a simplification for the
defer_buildlogic, but also touches a few other things intype_adapter.py.We are removing the
experimental_flag associated withdefer_buildforTypeAdapterin v2.10, so I think we should also go ahead and simplify this logic now so that we can avoid breaking changes in future releases.Closes#10632
Main changes:
frame_depthwrappersrebuild()function to type adapter instances to mimick themodel_rebuild()pattern onBaseModelscore_schema,validator, andserializerback to simple attributes rather than properties - this is more consistent with that ofBaseModels - said properties silently fail toMock...structure son schema build if annotations cannot be evaluated.defer_buildandmodel_configproperties, not functionsNote, there's a nice little perf boost here :)