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

feat: add support for NamedTuple and TypedDict types #2216

Merged
merged 29 commits into from Feb 13, 2021

Conversation

PrettyWood
Copy link
Member

@PrettyWood PrettyWood commented Dec 22, 2020

Change Summary

Screen Shot 2020-12-23 at 1 58 37 AM

Also expose two utils

  • create_model_from_namedtuple
  • create_model_from_typeddict

Screen Shot 2021-01-17 at 11 57 29 PM

Related issue number

fix #760
fix #1324
fix #1430

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)

@codecov
Copy link

codecov bot commented Dec 22, 2020

Codecov Report

Merging #2216 (1e60f5a) into master (13a5c7d) will decrease coverage by 0.11%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##            master    #2216      +/-   ##
===========================================
- Coverage   100.00%   99.88%   -0.12%     
===========================================
  Files           21       23       +2     
  Lines         4199     4405     +206     
  Branches       854      886      +32     
===========================================
+ Hits          4199     4400     +201     
- Misses           0        5       +5     
Impacted Files Coverage Δ
pydantic/annotated_types.py 100.00% <100.00%> (ø)
pydantic/fields.py 100.00% <100.00%> (ø)
pydantic/schema.py 100.00% <100.00%> (ø)
pydantic/typing.py 100.00% <100.00%> (ø)
pydantic/validators.py 100.00% <100.00%> (ø)
pydantic/types.py 100.00% <0.00%> (ø)
pydantic/networks.py 100.00% <0.00%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13a5c7d...1e60f5a. Read the comment docs.

@PrettyWood PrettyWood changed the title feat: add support for namedtuple type feat: add support for NamedTuple and TypedDict types Dec 23, 2020
@PrettyWood PrettyWood marked this pull request as ready for review December 23, 2020 02:34
Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

This looks awesome, would be great if we could have a proper example in the docs.

`typing.Dict`
: see [Typing Iterables](#typing-iterables) below for more detail on parsing and validation

`subclass of typing.TypedDict`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`subclass of typing.TypedDict`
`typing.TypedDict`

I think the subclass is implicit

Copy link
Member Author

Choose a reason for hiding this comment

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

It used to be the case for me too but I now tend to disagree since we added the support of enum.Enum to have a valid enum instance. We could someday add typing.TypedDict just to have a valid TypedDict instance for example. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

I would still prefer to remove subclass of and instead be explicity when we mean the actual type, like Enum.

@@ -85,9 +85,17 @@ with custom properties and validation.
`typing.Tuple`
: see [Typing Iterables](#typing-iterables) below for more detail on parsing and validation

`subclass of typing.NamedTuple (or collections.namedtuple)`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`subclass of typing.NamedTuple (or collections.namedtuple)`
`typing.NamedTuple` (or `collections.namedtuple`)

again I think the "subclass of..." is implicit

def is_typed_dict_type(type_: Type[Any]) -> bool:
from .utils import lenient_issubclass

return lenient_issubclass(type_, dict) and getattr(type_, '__annotations__', None)
Copy link
Member

Choose a reason for hiding this comment

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

Humm, I wonder if this is a water tight check. I can't think of a better solution though.

Copy link
Member Author

@PrettyWood PrettyWood Jan 1, 2021

Choose a reason for hiding this comment

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

Currently there is no easier way afaik. There will be a public method in 3.10 though

pydantic/validators.py Outdated Show resolved Hide resolved
pydantic/validators.py Outdated Show resolved Hide resolved
@@ -1425,3 +1434,90 @@ class M(BaseModel):
a: int

get_type_hints(M.__config__)


def test_named_tuple():
Copy link
Member

Choose a reason for hiding this comment

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

I know test files are a mess, but please can you move this to somewhere else. I try to keep test_main.py for the most vanilla cases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing! TBH I didn't know it was for vanilla cases. Maybe a module docstring on top of the module could help. I'll try to find a better place for these bad boys

Copy link
Member Author

Choose a reason for hiding this comment

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

I created a new dedicated file test_annotated_types. Suggestions welcome if you don't like it

@PrettyWood
Copy link
Member Author

Ok I added a dedicated section in the doc for both annotated types and took your remarks into account except the subclass thing (cf my remark). I feel like it's ready for another review :)

@RobertCraigie
Copy link

This looks great, I'll be putting it to good use, thanks!

However there is an issue that this does not respect subclassing, for example, the following will raise a ValidationError that the name field is missing.

class OptionalUser(TypedDict, total=False):
    name: str

class User(OptionalUser):
    id: int

class Model(BaseModel):
    user: User

Model(user={'id': 1})

I had a quick look to try and fix this issue but couldn't find a complete solution.

There are __required_keys__ and __optional_keys__ attributes but they are not guaranteed to be present on all versions and we cannot calculate the optional / required keys ourselves by traversing the mro as it is discarded upon class creation.

If there is no universal solution for this issue then __required_keys__ and __optional_keys__ should be respected if they are present.

@PrettyWood
Copy link
Member Author

PrettyWood commented Jan 11, 2021

Hi @RobertCraigie
First let me thank you for testing the code! It's so valuable to have feedback before a feature is actually merged.
You're right I didn't get that. My first implementation used __required_keys__ indeed but I got issues with the CI with older python versions that didn't support it.
I guess we could simpy use TypedDict from typing_extensions for python 3.9- if possible, which implements __required_keys__ and __optional_keys__ for python 3.6+. But it means the code will behave differently with and without the extension, which I don't like. Maybe we could add a warning in this case?

EDIT: I added a commit. Feedback welcome :)

@RobertCraigie
Copy link

RobertCraigie commented Jan 11, 2021

Glad I could help!

I would also like to be able to explicitly forbid passing extra keys, is this feasible?

@PrettyWood
Copy link
Member Author

@RobertCraigie Sure easy peasy ! :) Just needed to pass down the config like I did for dataclass.
I didn't pass it down to NamedTuple though as I feel like it makes no sense. It can probably be done in a future PR if needed.


if TYPE_CHECKING:

class TypedDict(Dict[str, Any]):
Copy link
Member Author

Choose a reason for hiding this comment

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

We could use typing_extensions.TypedDict for python < 3.9 or typing.TypedDict for 3.9+ but we would need to add typing_extensions in linting requirements.

@@ -4,5 +4,5 @@ Cython==0.29.21;sys_platform!='win32'
devtools==0.6.1
email-validator==1.1.2
dataclasses==0.6; python_version < '3.7'
typing-extensions==3.7.4.1; python_version < '3.8'
typing-extensions==3.7.4.3; python_version < '3.9'
Copy link
Member Author

Choose a reason for hiding this comment

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

Had to bump this to have the good version of TypedDict for python < 3.9

@Thorbenl
Copy link

Thorbenl commented Feb 5, 2021

Is there any timeframe for when we can expect this to get released? :)

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

this is going to be f***ing awesome. Can't wait for it.

Just a few small things.

`typing.Dict`
: see [Typing Iterables](#typing-iterables) below for more detail on parsing and validation

`subclass of typing.TypedDict`
Copy link
Member

Choose a reason for hiding this comment

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

I would still prefer to remove subclass of and instead be explicity when we mean the actual type, like Enum.

docs/usage/types.md Outdated Show resolved Hide resolved
docs/usage/types.md Outdated Show resolved Hide resolved
@@ -16,6 +17,9 @@
# WARNING __all__ from .errors is not included here, it will be removed as an export here in v2
# please use "from pydantic.errors import ..." instead
__all__ = [
# annotated types utils
'create_model_from_namedtuple',
Copy link
Member

Choose a reason for hiding this comment

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

yes agree, perhaps we might want create_model_from which takes any of the three types, but we definitely want these standalone methods for performance.

Or we could go even further and extend create_model to take a single dataclass, named tuple or typed dict instead of fields?

pydantic/annotated_types.py Outdated Show resolved Hide resolved
pydantic/annotated_types.py Outdated Show resolved Hide resolved
pydantic/validators.py Outdated Show resolved Hide resolved
pydantic/validators.py Outdated Show resolved Hide resolved
@PrettyWood
Copy link
Member Author

@samuelcolvin Feedback taken except for the rewording on types.md.
I'm not really sure how to explain the Enum for example.
Would you mind update types.md for me please ? I'm sure it will be clearer than what I would come by with

@samuelcolvin
Copy link
Member

samuelcolvin commented Feb 12, 2021

@samuelcolvin Feedback taken except for the rewording on types.md.
I'm not really sure how to explain the Enum for example.
Would you mind update types.md for me please ? I'm sure it will be clearer than what I would come by with

Don't worry about it, the whole file needs rethinking, what's there is fine.

I'll merge tomorrow.

@samuelcolvin samuelcolvin merged commit c314f5a into pydantic:master Feb 13, 2021
@samuelcolvin
Copy link
Member

🎉 this is great, thank you so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants