Add support for unpacked `TypedDict` to type hint variadic keyword arguments in `ArgumentsValidator` #1451

Viicos · 2024-09-13T14:10:41Z

Change Summary

Part of pydantic/pydantic#9619.

Related issue number

Checklist

Unit tests for the changes exist
Documentation reflects the changes where applicable
Pydantic tests pass with this pydantic-core (except for expected changes)
My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

src/validators/var_kwargs.rs

codspeed-hq · 2024-09-16T14:13:52Z

CodSpeed Performance Report

Merging #1451 will not alter performance

_{Comparing kwargs-td (57467bd) with main (8c1a0da)}

Summary

✅ 155 untouched benchmarks

Viicos · 2024-09-17T12:58:49Z

src/validators/arguments.rs

+            None => None,
+        };
+
+        if var_kwargs_mode == VarKwargsMode::UnpackedTypedDict && var_kwargs_validator.is_none() {


I'm wondering if we should also check that var_kwargs_validator is a TypedDictValidator?

I don't think you need to, given that you have the conditional checks in pydantic.

Making it a TypedDictValdiator statically (at build time) would allow for efficiency gains (by avoiding the dispatch via CombinedValidator).

But I wonder, are there cases where it can be wrapped in a function-after validator (e.g. model_validator)?

Making it a TypedDictValdiator statically (at build time) would allow for efficiency gains (by avoiding the dispatch via CombinedValidator).

The thing is (as described here) var_kwargs_validator is used for both uniform and unpacked-typed-dict modes.

But I wonder, are there cases where it can be wrapped in a function-after validator (e.g. model_validator)?

only config can be attached to typed dicts iirc, and it still results in a typed dict schema.

You could have

#[derive(Debug, PartialEq)] enum VarKwargsMode { Uniform(CombinedValidator), UnpackedTypedDict(TypedDictValidator), }

... and replace the FromStr implementation with a bespoke function like from_string_and_validator or similar.

Will leave this as an optimization for later, going to approve for now 👍

src/validators/arguments.rs

python/pydantic_core/core_schema.py

sydney-runkle

Looking great thus far! A couple of rust syntax suggestions + potentially one constant change.

python/pydantic_core/core_schema.py

sydney-runkle · 2024-09-17T16:48:35Z

python/pydantic_core/core_schema.py

+        var_kwargs_mode: The validation mode to use for variadic keyword arguments. If `'single'`, every value of the
+            keyword arguments will be validated against the `var_kwargs_schema` schema. If `'unpacked-typed-dict'`,
+            the `schema` argument must be a [`typed_dict_schema`][pydantic_core.core_schema.typed_dict_schema]


What if instead of single we did uniform? Don't want to bikeshed here too much, but I was a bit confused by single on its own.

Yes I'm not too happy with name either. uniform seems better, although there might be an even more explicit name, I'll think about it.

uniform seems good to me 👍

I wonder about whether to handle uniform via the "extras" of the typed dict schema (if such a thing exists) and have simpler code here (i.e. no need for mode, just always require typed dict schema). Maybe a little breaking, though?

There's a extras_schema key in the TypedDictSchema, but it is not used currently. For Pydantic models, we support:

class Model(BaseModel): __pydantic_extra__: dict[str, int]

But we don't for typed dicts. I believe it's best to wait for https://peps.python.org/pep-0728/, this will probably completely change how we handle extra keys for typed dicts currently.

Sounds good. We should probably start validating extra keys as well, just a side note :).

sydney-runkle · 2024-09-17T16:52:55Z

src/validators/arguments.rs

+            None => None,
+        };
+
+        if var_kwargs_mode == VarKwargsMode::UnpackedTypedDict && var_kwargs_validator.is_none() {


I don't think you need to, given that you have the conditional checks in pydantic.

src/validators/arguments.rs

Co-authored-by: Sydney Runkle <54324534+sydney-runkle@users.noreply.github.com>

davidhewitt

Overall LGTM with some brief thoughts...

davidhewitt · 2024-09-18T11:04:29Z

python/pydantic_core/core_schema.py

+        var_kwargs_mode: The validation mode to use for variadic keyword arguments. If `'single'`, every value of the
+            keyword arguments will be validated against the `var_kwargs_schema` schema. If `'unpacked-typed-dict'`,
+            the `schema` argument must be a [`typed_dict_schema`][pydantic_core.core_schema.typed_dict_schema]


uniform seems good to me 👍

src/validators/arguments.rs

davidhewitt · 2024-09-18T11:13:31Z

src/validators/arguments.rs

+            None => None,
+        };
+
+        if var_kwargs_mode == VarKwargsMode::UnpackedTypedDict && var_kwargs_validator.is_none() {


Making it a TypedDictValdiator statically (at build time) would allow for efficiency gains (by avoiding the dispatch via CombinedValidator).

But I wonder, are there cases where it can be wrapped in a function-after validator (e.g. model_validator)?

src/validators/arguments.rs

davidhewitt · 2024-09-18T11:18:17Z

python/pydantic_core/core_schema.py

+        var_kwargs_mode: The validation mode to use for variadic keyword arguments. If `'single'`, every value of the
+            keyword arguments will be validated against the `var_kwargs_schema` schema. If `'unpacked-typed-dict'`,
+            the `schema` argument must be a [`typed_dict_schema`][pydantic_core.core_schema.typed_dict_schema]


I wonder about whether to handle uniform via the "extras" of the typed dict schema (if such a thing exists) and have simpler code here (i.e. no need for mode, just always require typed dict schema). Maybe a little breaking, though?

Co-authored-by: David Hewitt <david.hewitt@pydantic.dev>

sydney-runkle

Great work @Viicos, looking forward to having this feature!

python/pydantic_core/core_schema.py

sydney-runkle · 2024-09-20T16:28:08Z

src/validators/arguments.rs

+            None => None,
+        };
+
+        if var_kwargs_mode == VarKwargsMode::UnpackedTypedDict && var_kwargs_validator.is_none() {


Will leave this as an optimization for later, going to approve for now 👍

Viicos commented Sep 16, 2024

View reviewed changes

src/validators/var_kwargs.rs Outdated Show resolved Hide resolved

Viicos force-pushed the kwargs-td branch from 08969cd to 1d84ff5 Compare September 16, 2024 09:27

Viicos mentioned this pull request Sep 16, 2024

Add support for unpacked TypedDict to type hint variadic keyword arguments with @validate_call pydantic/pydantic#10416

Merged

5 tasks

Add support for TypedDict for **kwargs

5969925

Viicos force-pushed the kwargs-td branch from 0e96f29 to 9cab63c Compare September 17, 2024 12:53

Viicos commented Sep 17, 2024

View reviewed changes

src/validators/arguments.rs Outdated Show resolved Hide resolved

Flatten everything under the arguments validator

1149fe4

Viicos force-pushed the kwargs-td branch from 9cab63c to 1149fe4 Compare September 17, 2024 13:01

Viicos changed the title ~~Add support for TypedDict for **kwargs~~ Add support for unpacked TypedDict to type hint variadic keyword arguments with @validate_call Sep 17, 2024

Viicos changed the title ~~Add support for unpacked TypedDict to type hint variadic keyword arguments with @validate_call~~ Add support for unpacked TypedDict to type hint variadic keyword arguments in the ArgumentsValidator Sep 17, 2024

Viicos commented Sep 17, 2024

View reviewed changes

python/pydantic_core/core_schema.py Show resolved Hide resolved

Viicos changed the title ~~Add support for unpacked TypedDict to type hint variadic keyword arguments in the ArgumentsValidator~~ Add support for unpacked TypedDict to type hint variadic keyword arguments in ArgumentsValidator Sep 17, 2024

Viicos marked this pull request as ready for review September 17, 2024 13:06

sydney-runkle requested changes Sep 17, 2024

View reviewed changes

pydantic-hooky bot added awaiting author revision labels Sep 17, 2024

pydantic-hooky bot assigned Viicos Sep 17, 2024

Feedback

b9dbb34

Co-authored-by: Sydney Runkle <54324534+sydney-runkle@users.noreply.github.com>

davidhewitt reviewed Sep 18, 2024

View reviewed changes

Viicos and others added 2 commits September 18, 2024 14:24

Feedback

a5206e5

Co-authored-by: David Hewitt <david.hewitt@pydantic.dev>

Always validate if in unpack typed dict mode

99d3303

sydney-runkle approved these changes Sep 20, 2024

View reviewed changes

sydney-runkle mentioned this pull request Sep 20, 2024

Optimize VarKwargsMode by skipping CombinedValidator build #1457

Open

Update python/pydantic_core/core_schema.py

57467bd

sydney-runkle enabled auto-merge (squash) September 20, 2024 16:30

sydney-runkle merged commit f3f436e into main Sep 20, 2024
28 of 29 checks passed

sydney-runkle deleted the kwargs-td branch September 20, 2024 16:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for unpacked `TypedDict` to type hint variadic keyword arguments in `ArgumentsValidator` #1451

Add support for unpacked `TypedDict` to type hint variadic keyword arguments in `ArgumentsValidator` #1451

Viicos commented Sep 13, 2024

codspeed-hq bot commented Sep 16, 2024 •

edited

Loading

Viicos Sep 17, 2024

sydney-runkle Sep 17, 2024

davidhewitt Sep 18, 2024

Viicos Sep 18, 2024

davidhewitt Sep 18, 2024

sydney-runkle Sep 20, 2024

sydney-runkle Sep 20, 2024

sydney-runkle left a comment

sydney-runkle Sep 17, 2024

Viicos Sep 17, 2024

davidhewitt Sep 18, 2024

davidhewitt Sep 18, 2024

Viicos Sep 18, 2024

sydney-runkle Sep 20, 2024

sydney-runkle Sep 17, 2024

davidhewitt left a comment

davidhewitt Sep 18, 2024

davidhewitt Sep 18, 2024

davidhewitt Sep 18, 2024

sydney-runkle left a comment

sydney-runkle Sep 20, 2024

Add support for unpacked TypedDict to type hint variadic keyword arguments in ArgumentsValidator #1451

Add support for unpacked TypedDict to type hint variadic keyword arguments in ArgumentsValidator #1451

Conversation

Viicos commented Sep 13, 2024

Change Summary

Related issue number

Checklist

codspeed-hq bot commented Sep 16, 2024 • edited Loading

Merging #1451 will not alter performance

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sydney-runkle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidhewitt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sydney-runkle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add support for unpacked `TypedDict` to type hint variadic keyword arguments in `ArgumentsValidator` #1451

Add support for unpacked `TypedDict` to type hint variadic keyword arguments in `ArgumentsValidator` #1451

codspeed-hq bot commented Sep 16, 2024 •

edited

Loading