Add support for nested generics by dmontagu · Pull Request #1104 · pydantic/pydantic

dmontagu · 2019-12-16T22:14:00Z

Change Summary

Modifies GenericModel to behave more similarly to standard python generics, including the ability to reference typevars in nested GenericModels, and to make use of partial specifications with new typevars.

There are a couple outstanding issues:

As this PR stands, an error will be raised if you try to instantiate a GenericModel after just passing TypeVars as the parameters. Currently, I believe this is allowed, and will perform the same logic that would happen if you were to use a TypeVar as a field annotation elsewhere (e.g., for purposes of a constraint).
- I believe the right (backwards-compatible) approach here is to treat unspecified parameters the same way we would treat it as if the TypeVar was used as the annotation. I think this is similar to what we do for Dict, List, etc., since those TypeVars have no constraints and so are basically equivalent to Any.
- This should be easy to implement, but I wanted to get confirmation before putting in more effort. In particular, I think the "right" implementation would be to allow instantiating before you've provided any concrete types for the parameters, similar to how Dict, etc., work. But right now that raises an error. I personally would consider this a fix rather than a breaking change, but I want to reach consensus before moving forward.
- (It should also be easy to keep the current behavior, but allow instantiation after explicitly specifying TypeVar parameters, I just think that is a wrong API choice -- the question is whether preserving the current behavior is sufficiently worthwhile.)
The generated names come out a little weird when you make use of partially-specified models. It would be nice to fix this, but since 1) this is already an edge case, 2) type naming is usually not critical, and 3) we already allow you to override __concrete_name__, I am happy to leave this to users for now.

Related issue number

Closes #967

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 · 2019-12-17T11:06:38Z

Codecov Report

Merging #1104 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #1104   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          20      20           
  Lines        3450    3463   +13     
  Branches      667     672    +5     
======================================
+ Hits         3450    3463   +13

Impacted Files	Coverage Δ
pydantic/generics.py	`100% <100%> (ø)`	⬆️

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 1c49a3f...c4df92d. Read the comment docs.

samuelcolvin

I'm fine with the generated names being weird, but can we have an example in a test so we're clear.

As this PR stands, an error will be raised if you try to instantiate a GenericModel after just passing TypeVars as the parameters

I'm afraid I don't know exactly what you mean. Could you add an example here so I (and anyone else coming to this PR) is clear what you mean. My suspicion is that, as you say, this can reasonably considered a bug fix, not a change in functionality so is fine here. But's document it in a test and perhaps even in the documentation so everything is clear.

dmontagu · 2020-01-10T01:15:36Z

    __slots__ = ()
    __concrete__: ClassVar[bool] = False

-    def __new__(cls, *args: Any, **kwargs: Any) -> Any:


(This was only necessary to prevent instantiation with unspecified parameters.)

dmontagu · 2020-01-10T01:16:35Z

+    partial_model = Model[int, BT]
+    assert partial_model.__name__ == 'Model[int, BT]'
+    concrete_model = partial_model[str]
+    assert concrete_model.__name__ == 'Model[int, BT][str]'


This is what I mean by the names are kind of weird.

It would be a (perhaps surprisingly) involved refactor to make it generate the names properly (by that I mean showing Model[int, str] instead of Model[int, BT][str]). Not impossible though if you'd prefer we make it work.

dmontagu · 2020-01-10T01:24:04Z

+
+
+@skip_36
+def test_partial_specification_instantiation_bounded():


This test and the one below demonstrate that the typevars for (partially-)parametrized models are respected properly. This is a big part of why I think it's okay to drop the check for parametrization in __new__.

samuelcolvin · 2020-01-10T20:10:41Z

I think this looks fine, but I'm not going to pretend I've followed every line of the discussion.

Does it need any updates to docs? Otherwise can it be merged?

dmontagu · 2020-01-10T20:20:54Z

I think the docs should be updated, but I think it can be a small update. Give me 15 mins and I'll respond here with an estimate of how long it will take to finish.

dmontagu · 2020-01-10T20:49:03Z

@samuelcolvin Okay, I've added docs, let me know if you want any changes.

samuelcolvin · 2020-01-10T21:06:59Z

awesome, thank you.

* Add support for nested generics * Allow instantiation of unparameterized generics * Add better more partial instantiation tests * Add changes * Add docs

Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

dmontagu mentioned this pull request Dec 16, 2019

Nested generic models #967

Closed

dmontagu force-pushed the nested-generics branch from ca5f73d to fe5c43c Compare December 17, 2019 11:06

samuelcolvin reviewed Dec 18, 2019

View reviewed changes

Comment thread pydantic/generics.py

dmontagu mentioned this pull request Dec 30, 2019

Restrict GenericModel to certain data types #1128

Closed

David Montague added 2 commits January 9, 2020 17:04

Add support for nested generics

f8c8faf

Allow instantiation of unparameterized generics

e2fa3d5

dmontagu force-pushed the nested-generics branch from fe5c43c to e2fa3d5 Compare January 10, 2020 01:14

dmontagu commented Jan 10, 2020

View reviewed changes

Add better more partial instantiation tests

abdb1fc

dmontagu force-pushed the nested-generics branch from 2704265 to abdb1fc Compare January 10, 2020 01:32

Add changes

190ef6c

Add docs

c4df92d

dmontagu force-pushed the nested-generics branch from 971ac39 to c4df92d Compare January 10, 2020 20:47

samuelcolvin merged commit dbc044e into pydantic:master Jan 10, 2020

alexdrydew pushed a commit to alexdrydew/pydantic that referenced this pull request Dec 23, 2023

Bump serde from 1.0.190 to 1.0.193 (pydantic#1104)

a5e0e09

Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for nested generics#1104

Add support for nested generics#1104
samuelcolvin merged 5 commits intopydantic:masterfrom
dmontagu:nested-generics

dmontagu commented Dec 16, 2019 •

edited

Loading

Uh oh!

codecov Bot commented Dec 17, 2019 •

edited

Loading

Uh oh!

samuelcolvin left a comment

Uh oh!

Uh oh!

dmontagu Jan 10, 2020

Uh oh!

dmontagu Jan 10, 2020 •

edited

Loading

Uh oh!

dmontagu Jan 10, 2020 •

edited

Loading

Uh oh!

samuelcolvin commented Jan 10, 2020

Uh oh!

dmontagu commented Jan 10, 2020 •

edited

Loading

Uh oh!

dmontagu commented Jan 10, 2020

Uh oh!

samuelcolvin commented Jan 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants



		@skip_36
		def test_partial_specification_instantiation_bounded():

Uh oh!

Conversation

dmontagu commented Dec 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Summary

Related issue number

Checklist

Uh oh!

codecov Bot commented Dec 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

samuelcolvin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmontagu Jan 10, 2020

Choose a reason for hiding this comment

Uh oh!

dmontagu Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmontagu Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samuelcolvin commented Jan 10, 2020

Uh oh!

dmontagu commented Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmontagu commented Jan 10, 2020

Uh oh!

samuelcolvin commented Jan 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dmontagu commented Dec 16, 2019 •

edited

Loading

codecov Bot commented Dec 17, 2019 •

edited

Loading

dmontagu Jan 10, 2020 •

edited

Loading

dmontagu Jan 10, 2020 •

edited

Loading

dmontagu commented Jan 10, 2020 •

edited

Loading