Simplify handling of typing.Annotated in GenerateSchema#6887
Conversation
| list(self.defs.definitions.values()), | ||
| ) | ||
|
|
||
| def _add_js_function(self, metadata_schema: CoreSchema, js_function: Callable[..., Any]) -> None: |
There was a problem hiding this comment.
this function just moved, no changes were made
Deploying with
|
| Latest commit: |
8c6d630
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2f488098.pydantic-docs2.pages.dev |
| Branch Preview URL: | https://simplify-annotated-core-sche.pydantic-docs2.pages.dev |
| if js_function not in pydantic_js_functions: | ||
| pydantic_js_functions.append(js_function) | ||
|
|
||
| def _generate_schema_for_type( |
There was a problem hiding this comment.
I eliminated this method since the only place it was called was in generate_schema, and with the removal of the handling of _annotated_schema there, generate_schema was just immediately calling this function with the exact same signature and arguments passing through without modification.
| if isinstance(obj, type(Annotated[int, 123])): | ||
| schema = transform_inner_schema(self._annotated_schema(obj)) |
There was a problem hiding this comment.
This branch no longer seemed necessary now that _generate_schema handles Annotated properly (and Annotated doesn't hit any of the other property extractions as far as I can tell). So I just eliminated the branch here, that's all that's happening in this chunk.
| from ._schema_generation_shared import GetJsonSchemaFunction | ||
|
|
||
| _SUPPORTS_TYPEDDICT = sys.version_info >= (3, 12) | ||
| _AnnotatedType = type(Annotated[int, 123]) |
There was a problem hiding this comment.
Repeated calls to type and Annotated.__class_getitem__ while building schemas seemed unnecessary/inefficient, but I didn't profile this or anything. Probably negligible overhead but just ... rubbed me the wrong way reading isinstance(obj, type(Annotated[int, 123])) in a frequently-executed loop (at least during model creation) lol. Can revert this change if preferred.
Closes #6884
@adriangb It seems to me some of the cruft removed in this PR might have been left around from previous fiddling that has been rendered unnecessary. But if you see issues with this we can revert some of the other changes; I'll note that an alternative diff that also closes #6884 is just:
But looking into the code, it seemed to me this was a "safer" solution. But I think you know this code best at this point.