Use the typing-inspection library
#11479
Conversation
This commit removes most of the introspection functions defined in the `_typing_extra` module, and replaces usage with the ones from `typing-inspection` instead. Small improvements/changes also have been applied.
Deploying pydantic-docs with
|
| Latest commit: |
3701115
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4181d96a.pydantic-docs.pages.dev |
| Branch Preview URL: | https://typing-inspection.pydantic-docs.pages.dev |
CodSpeed Performance ReportMerging #11479 will not alter performanceComparing Summary
|
4170deb to
b343465
Compare
This error will be used as a wrapper to the `ForbiddenQualifier` exception from the `typing-inspection` library, with a clear error message.
This simplifies the logic on our end: we don't need to manually unwrap the annotated forms and qualifiers. The `inspect_annotation()` is also more robust, and support more cases (as per the added test).
The typed dict schema logic is left unchanged, as the next commit will refactor how required keys are detected, and will also add logic related to `ReadOnly`.
Instead of trying to find `Required`/`NotRequired` qualifiers (which could be nested under `Annotated` forms and/or `ReadOnly`), we rely on the `FieldInfo._qualifiers` private attribute, populated from the `inspect_annotation()` result which is guaranteed to correctly handle these cases. We also now don't error if we encounter `ReadOnly`, but raise a warning stating we don't enforce immutability of the dict item.
The source is provided, and the logic to detect final fields with a default value is moved *after* the `FieldInfo` instance is created. The reason to do so is to again support more edge cases, e.g. When the annotation was wrapped with `Annotated` (e.g. `Annotated[Final[int], ...] = 1`. This previously wasn't considered and as such wasn't treated as a classvar. As per the linked issue in this PR, this led to an inconsistency, so while technically this can be considered as a breaking change, we'll consider this as a bug.
882f11d to
ffc1f3b
Compare
pydantic/fields.py
Outdated
| default.metadata += annotation_metadata | ||
| default = default.merge_field_infos( | ||
| *[x for x in annotation_metadata if isinstance(x, FieldInfo)], default, annotation=default.annotation | ||
| # TODO infer from the default |
There was a problem hiding this comment.
This can wait for v3, as if the field has a default it is currently treated as a class var anyway. This todo is thus linked in #11119.
ffc1f3b to
340d32d
Compare
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f'The annotation {_repr.display_as_type(annotation)!r} contains the {self._qualifier_repr_map[qualifier]!r} ' | ||
| f'type qualifier, which is invalid in the context it is defined.' | ||
| ), | ||
| code=None, |
|
|
||
| from . import _repr | ||
| from ._typing_extra import is_generic_alias, is_type_alias_type | ||
| from ._typing_extra import is_generic_alias |
There was a problem hiding this comment.
Why is is_generic_alias not included in typing_inspection?
There was a problem hiding this comment.
is_generic_alias does an isinstance() check against typing._GenericAlias (e.g. List[int] is an instance of such a class), which isn't documented and technically private (although I don't think it will change). So it's best to avoid relying on it.
It is also a footgun as while is_generic_alias() works for all parameterized typing objects, it doesn't check for new unions (is_generic_alias(int | str) == False, but is_generic_alias(Union[int, str]) == True). For instance, I'm not sure we expected new unions to be skipped here:
pydantic/pydantic/_internal/_core_utils.py
Lines 66 to 74 in acb0f10
Similarly, I've used this function here as a way to check for type[list[int]] forms (here type_param is list[int]):
pydantic/pydantic/_internal/_generate_schema.py
Lines 1711 to 1715 in acb0f10
This would also match type[Union[int, str]], which we actually want to support! Thankfully there's a specific check for unions just before, but this could easily be missed.
I think there are still valid use cases where you want to check if something is a generic alias (and by that I don't mean isinstance(obj, (types.GenericAlias, typing._GenericAlias), but if the obj is a parameterized generic class -- excluding unions, typing special forms like Literal, Annotated, etc), but it's probably best to rely on get_origin() and the typing_objects check functions.
| @@ -286,7 +287,7 @@ def _warn_on_nested_alias_in_annotation(ann_type: type[Any], ann_name: str) -> N | |||
| args = getattr(ann_type, '__args__', None) | |||
| if args: | |||
| for anno_arg in args: | |||
| if _typing_extra.is_annotated(anno_arg): | |||
| if typing_objects.is_annotated(get_origin(anno_arg)): | |||
There was a problem hiding this comment.
I presume it's expected that the origin is always fetched before feeding into a typing objects check?
There was a problem hiding this comment.
Yes, as per the guide, for performance reasons and also to have a cleaner approach, you should call get_origin() once, and the use the typing_objects.is_smth(origin) functions to analyze the object.
In Pydantic, our is_smth() functions were already calling get_origin(), but then we can end up with patterns like:
if is_annotated(obj): # one get_origin() call under the hood
...
elif is_literal(obj): # two get_origin() calls now
...This was convenient as shown on this if _typing_extra.is_annotated() call, but can decrease performance overall.
There was a problem hiding this comment.
Yes, definitely good to minimize get_origin calls.
| @@ -1104,9 +1107,9 @@ def _match_generic_type(self, obj: Any, origin: Any) -> CoreSchema: # noqa: C90 | |||
| if schema is not None: | |||
| return schema | |||
|
|
|||
| if _typing_extra.is_type_alias_type(origin): | |||
| if typing_objects.is_typealiastype(origin): | |||
There was a problem hiding this comment.
Would is_type_alias_type make more sense?
There was a problem hiding this comment.
I wanted to avoid having the name depending on the capitalization of the object, and simply have is_<name>, but both make sense I think
There was a problem hiding this comment.
I'd prefer is_type_alias_type just bc it's easier to read, but it's up to you, doesn't really matter!
| @@ -297,15 +297,15 @@ def replace_types(type_: Any, type_map: Mapping[TypeVar, Any] | None) -> Any: | |||
| origin_type = getattr(typing, type_._name) | |||
| assert origin_type is not None | |||
|
|
|||
| if _typing_extra.origin_is_union(origin_type): | |||
| if any(_typing_extra.is_any(arg) for arg in resolved_type_args): | |||
| if is_union_origin(origin_type): | |||
There was a problem hiding this comment.
Why not just is_union?
There was a problem hiding this comment.
To not confuse it with typing_object.is_union(), which only checks for typing.Union (as it does for all the other functions in typing_objects), I've tried finding a better name but couldn't find any
There was a problem hiding this comment.
Discussed at standup today, could be changed for clarity on the typing-inspection end, but I don't think this PR needs to be held up because of it.
| core_schema_types: list[CoreSchemaOrFieldType] = _typing_extra.literal_values( | ||
| CoreSchemaOrFieldType # type: ignore | ||
| ) | ||
| core_schema_types: list[CoreSchemaOrFieldType] = list(get_literal_values(CoreSchemaOrFieldType)) |
There was a problem hiding this comment.
Why are we yielding from get_literal_values? Seems like we always wrap in a list?
There was a problem hiding this comment.
We (Pydantic) always wrap in a list, but others might not:
for elem in get_literal_values(Literal[1, 2]):
...There was a problem hiding this comment.
Doesn't strike me as necessary, but also, very low stakes, feel free to leave it as you wish
80ed389 to
343bef3
Compare
pydantic-settings relies on it.
2b21340 to
713c1bd
Compare
The library is incorrectly hooking into generic aliases.
3347fe3 to
3701115
Compare
Change Summary
Needs #11475 to be merged first.
Fixes #9904, fixes #11346, fixes #11510.
Related issue number
Checklist