Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update create_model() to support typing.Annotated as input #8947

Merged
merged 11 commits into from Mar 18, 2024

Conversation

wannieman98
Copy link
Contributor

@wannieman98 wannieman98 commented Mar 5, 2024

Change Summary

At the moment, dynamic model creation does not support taking input of typing.Annotated but instead only tuple(, ), taking in typing.Annotated allows using of complex input types for dynamic model creation. At the moment, typing.Annotated does not support construction information during runtime so we have to use a private class name to check whether the provided field_definition is a typing.Annotated. Also, I have rather decided that it is best to ignore any additional data types or metadata other than the first and second input variables because 1. first input variable is guaranteed to be a valid input type and 2. second input variable has to be related metadata of the first input variable.

Related issue number

Closes #8534

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @dmontagu

At the moment, dynamic model creation does not support taking input of typing.Annotated but instead only tuple(<type>, <default value>), taking in typing.Annotated allows using of comples input types for dynamic model creation.
@wannieman98 wannieman98 marked this pull request as ready for review March 5, 2024 07:58
Copy link

codspeed-hq bot commented Mar 5, 2024

CodSpeed Performance Report

Merging #8947 will not alter performance

Comparing wannieman98:wanyoo/issue-8534 (9dd7a14) with main (5c8afe6)

Summary

✅ 10 untouched benchmarks

@wannieman98
Copy link
Contributor Author

Please Review!

Seems like I cannot add labels related to the pull request, would be great if I can get assistance adding the according label.

Thanks!

pydantic/main.py Outdated Show resolved Hide resolved
tests/test_create_model.py Outdated Show resolved Hide resolved
@Viicos
Copy link
Contributor

Viicos commented Mar 5, 2024

Moving from #8947 (comment):

typing._AnnotatedAlias is an implementation detail, and is_annotated relies on get_origin which is the documented way of getting the unsubscripted version of the Annotated special form.

Instead, I would do something like:

        elif  _typing_extra.is_annotated(f_def):
            try:
                f_annotation = (
                    f_def.__origin__
                )  # __origin__ represents the annotation_type in Annotated[annotate_type, ...]

                f_value = f_def.__metadata__[
                    0
                ]  # Python Annotated requires at least one python variable to represent the annotation
            except ValueError as e:
                raise PydanticUserError(
                    'Field definitions should be a `typing.Annotation[type, definition]`.',
                    code='create-model-field-definitions',
                ) from e

The Python version check is not necessary, as is_annotated will safely return False if it is not an instance of Annotated. Plus people can make use of typing_extensions.Annotated (and is_annotated can handle both).

I'll note that accessing __origin__ 1 and __metadata__ is not documented, although I don't think the implementation will change anytime.

One way of doing it would be:

f_annotation, f_value = typing_extensions.get_args(f_def)

Considering Annotated is guaranteed to have a type and at least one metadata value. In Python 3.10, I get TypeError: Annotated[...] should be used with at least two arguments (a type and an annotation). when trying to do Annotated[int], but I'm not sure this is the case on 3.8/3.9.

Footnotes

  1. Not to be confused with the return value of get_origin (yeah this is weird). Annotated[int, ""].__origin__ returns int, while get_origin(Annotated[int, ""]) return the Annotated class.

@wannieman98
Copy link
Contributor Author

Moving from #8947 (comment):

typing._AnnotatedAlias is an implementation detail, and is_annotated relies on get_origin which is the documented way of getting the unsubscripted version of the Annotated special form.

Instead, I would do something like:

        elif  _typing_extra.is_annotated(f_def):
            try:
                f_annotation = (
                    f_def.__origin__
                )  # __origin__ represents the annotation_type in Annotated[annotate_type, ...]

                f_value = f_def.__metadata__[
                    0
                ]  # Python Annotated requires at least one python variable to represent the annotation
            except ValueError as e:
                raise PydanticUserError(
                    'Field definitions should be a `typing.Annotation[type, definition]`.',
                    code='create-model-field-definitions',
                ) from e

The Python version check is not necessary, as is_annotated will safely return False if it is not an instance of Annotated. Plus people can make use of typing_extensions.Annotated (and is_annotated can handle both).

I'll note that accessing __origin__ 1 and __metadata__ is not documented, although I don't think the implementation will change anytime.

One way of doing it would be:

f_annotation, f_value = typing_extensions.get_args(f_def)

Considering Annotated is guaranteed to have a type and at least one metadata value. In Python 3.10, I get TypeError: Annotated[...] should be used with at least two arguments (a type and an annotation). when trying to do Annotated[int], but I'm not sure this is the case on 3.8/3.9.

Footnotes

  1. Not to be confused with the return value of get_origin (yeah this is weird). Annotated[int, ""].__origin__ returns int, while get_origin(Annotated[int, ""]) return the Annotated class.

The typing.Annotated is introduced in Python 3.9 at PEP 593 and the syntax specifies it expects a valid type annotation and a metadata. That will be applicable for Python versions >= 3.9, and for Python versions < 3.9. Also, that seems to be applicable for python version 3.8 as the current typing.Annotated seems it has moved over without fundamental changes from typing_extensions.Annotated.

Thanks for pointing it out, it seems I can use the existing function for both typing_extensions.Annotated as well as typing.Annotated seems I just need version checks in testing.

@Viicos
Copy link
Contributor

Viicos commented Mar 5, 2024

seems I just need version checks in testing.

You can use typing_extensions.Annotated in tests so that you can drop the version check here as well (or maybe keep a test with the Python version check and typing.Annotated to be sure that both are supported ;))

Copy link
Member

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Requested some changes. Looking great overall!

tests/test_create_model.py Outdated Show resolved Hide resolved
tests/test_create_model.py Outdated Show resolved Hide resolved
pydantic/main.py Show resolved Hide resolved
pydantic/main.py Outdated Show resolved Hide resolved
@sydney-runkle
Copy link
Member

sydney-runkle commented Mar 17, 2024

@wannieman98,

If you're able to make these changes in the next few days, we can get this into the upcoming minor release! Ping me if you have any additional questions :). We really appreciate your contribution 🚀 !

@wannieman98
Copy link
Contributor Author

wannieman98 commented Mar 17, 2024

@wannieman98,

If you're able to make these changes in the next few days, we can get this into the upcoming minor release! Ping me if you have any additional questions :). We really appreciate your contribution 🚀 !

Hi @sydney-runkle, sorry for the late revision, I've resolved the comments please review once more! Excited to have it part of the release!

docs/errors/usage_errors.md Outdated Show resolved Hide resolved
pydantic/main.py Outdated Show resolved Hide resolved
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
Copy link
Member

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

These changes look great. Thanks for your awesome work on this!

@sydney-runkle sydney-runkle enabled auto-merge (squash) March 18, 2024 13:11
@sydney-runkle sydney-runkle merged commit 26a2f06 into pydantic:main Mar 18, 2024
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

create_model fails to use type annotation of typing.Annotated
4 participants