Add generic functionality #595

dmontagu · 2019-06-14T11:03:54Z

Change Summary

Adds some functionality around generics.

There are definitely some edge cases not perfectly handled yet, but I wanted to get your take on this before I invest more effort. A number of people from the FastAPI community seem interested in this functionality, but if you think it's going to be too hard to maintain then we can let it go.

For what it's worth, I think the create_model function is actually doing 90% of the work necessary to get this working.

Related issue number

#556

Checklist

Unit tests for the changes exist
Tests pass on CI and coverage remains at 100%
Documentation reflects the changes where applicable
HISTORY.rst has been updated
- if this is the first change since a release, please add a new section
- include the issue number or this pull request number #<number>
- include your github username @<whomever>

dmontagu · 2019-06-14T11:11:51Z

I think it would be hard to get this working in python 3.6 due to differences in the way generics work. But I could look into it if necessary.

samuelcolvin · 2019-06-14T11:26:34Z

I'll look at this further, but overall looks cool.

I don't think 3.6 matters, just 3.7 support for this is fine.

Please use single quotes, I'll do my best to add linting for single quotes somehow soon.

codecov · 2019-06-14T11:27:08Z

Codecov Report

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

@@          Coverage Diff          @@
##           master   #595   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines        2582   2544   -38     
  Branches      510    504    -6     
=====================================
- Hits         2582   2544   -38

samuelcolvin

Good work, basically this looks good to me.

pydantic/generics.py

tests/test_generics.py

pydantic/generics.py

dmontagu · 2019-06-14T22:09:07Z

pydantic/generics.py

+from pydantic.class_validators import gather_validators
+
+
+class GenericModel:


I figured GenericModel was an appropriate name but I can change it back if desired.

pydantic/generics.py

tests/test_generics.py

setup.py

drunkwcodes · 2019-06-15T00:00:26Z

Can it be some constraints on Generics, e.g. "not iterable"?
It is less expressive without type constraints and losts the purpose with type constraints.

dmontagu · 2019-06-15T01:36:38Z

Can it be some constraints on Generics, e.g. "not iterable"?
It is less expressive without type constraints and losts the purpose with type constraints.

@drunkwcodes I'm not sure exactly what you mean. The primary (intended) value here is that you can more easily generate lots of models with a different BaseModel (or not, but seems more likely to be useful with many BaseModels) as a specific attribute, without having to declare a separate model for each attribute-class you want to contain.

Think:

class Response(GenericModel, Generic[T])
    data: T
    error: ErrorModel

then all you have to do is write

Response[int]
Response[str]
Response[MyModel]

and you get the corresponding type without having to declare it.

drunkwcodes · 2019-06-15T02:46:00Z

@dmontagu It is useful when you want to construct a list with *args and ensure there is no iterable in them.

dmontagu · 2019-06-15T08:28:39Z

Hmm, I didn't notice this because mypy doesn't run over the tests by default (which is where most of my usages were), but it looks like mypy doesn't like type variables if you don't explicitly inherit from Generic (or another typing class). So it looks like it may be necessary to change back to

class Model(GenericModel, Generic[T, S]):
    ...

I'll get that version working again, with all of these improvements.

dmontagu · 2019-06-15T09:24:54Z

Okay, everything worked out nicer than expected:

I was able to inherit from BaseModel without any problems, which helps mypy with all the built-in BaseModel methods.
If we want to support differing numbers of type variables (which I think we should), I think it's necessary that generic classes also inherit from Generic[T] or similar. This appears to be imposed by typing since you can't create your own custom variadic generic type yet. (This is an open issue in python/typing.)

Overall I'm very pleased with how neatly this is all fitting together, I expected typing to get in the way a lot more.

pydantic/generics.py

drunkwcodes · 2019-06-15T09:59:30Z

@dmontagu typing works sometimes. It depends.

drunkwcodes

What if introducing Not as another _SpecialForm and make GenericModel recognize it?

drunkwcodes · 2019-06-15T10:26:34Z

pydantic/generics.py

+    __slots__ = ()
+
+    def __new__(cls, *args: Any, **kwargs: Any) -> NoReturn:
+        if cls is GenericModel:


Does it mean it is not abstract and still can not be initialized without subclassing and not useful without implementing the function by users themselves?

samuelcolvin · 2019-06-15T11:59:41Z

Looks good to me, please can you:

update history
add a section to docs
"Hmm, I didn't notice this because mypy doesn't run over the tests by default..." add to the mypy external tests (tests/mypy_test_success.py) to check that Generic works with mypy

@drunkwcodes I'm afraid I don't understand what you're asking/concerned about. Probably best to give a code example to demonstrate your point. If you're not sure how something will work, it would be good if you checked out this PR and ran code yourself to check, then reported back here if you've found a problem.

drunkwcodes · 2019-06-15T13:04:28Z

@samuelcolvin The test_generic() itself is expressive enough to show how a type checking function can be faked for other implementors.

pydantic/generics.py

tiangolo · 2019-06-15T21:50:11Z

Great job @dmontagu! Impressive 👏👏🎉

Even the docs are great! 📝🚀

dmontagu · 2019-06-16T21:54:31Z

One thing that might be problematic is that I am placing [ and ] in the name of the created model. This makes all the associated validation messages cleaner / easier to parse, but I could see it causing unforeseen issues with the schema-related features due to having special characters (since you won't get special characters in the name of any model declared e.g. by inheriting from BaseModel).

In particular, @tiangolo I could see this causing issues with the OpenAPI spec generated by FastAPI. Is there anything to worry about here? (Maybe there's a config setting that let's you override the name of the model used in the spec? I'm a little out of my depth here.)

dmontagu · 2019-06-18T06:23:42Z

Okay, I tested it out, and there don't seem to be any problems with FastAPI. However, the generated names (which currently may include [, ], , ., and , violate the openapi spec, and prevent the use of openapi-generator because they don't meet the regex ^[a-zA-Z0-9\.\-_]+$.

dmontagu · 2019-06-18T06:38:23Z

pydantic/schema.py

@@ -346,6 +347,7 @@ def get_model_name_map(unique_models: Set[Type['BaseModel']]) -> Dict[Type['Base
    conflicting_names: Set[str] = set()
    for model in unique_models:
        model_name = model.__name__
+        model_name = re.sub(r"[^a-zA-Z0-9.\-_]", "_", model_name)


@samuelcolvin I expect this line may need to be removed / changed before merging, but I tested it, and adding this regex substitution fixes all problems with generic class names for FastAPI+openapi-generator (and I imagine for most json-schema-based tools). As far as I can tell, it isn't possible to generate a name that would be modified by this regex substitution in the normal course of pydantic usage (i.e., inheriting from BaseModel), including would-be-substituted characters is a violation of the open api spec (and presumably others), and I am having a hard time imagining a scenario where you would want those characters in your model name anyway.

From what I can tell, this substitution would also have little-to-no impact on performance given it's only involved in schema generation and is a fairly cheap regex.

I'm reluctant to change the naming scheme for generic classes because the repr and error messages are so clear. At the same time, I think json schemas are one of the most natural contexts for the use of generics (and my intended use is specifically with openapi / openapi-generator), so I think it would be unfortunate if they were slightly broken for this use case.

I'd be interested in your thoughts.

LGTM

This function is mainly/only used to generate the JSON Schemas (and it's reused by FastAPI).

I think it would be safe/ideal to apply this. And I see you are using the same regex in the OpenAPI spec ✔️

dmontagu · 2019-06-18T07:21:19Z

pydantic/generics.py

@@ -56,6 +56,8 @@ def __class_getitem__(  # type: ignore
        created_model.Config = cls.Config
        created_model.__concrete = True  # type: ignore
        _generic_types_cache[(cls, params)] = created_model
+        if len(params) == 1:
+            _generic_types_cache[(cls, params[0])] = created_model
        return created_model


You need to have the non-tuple version in the cache so cache-access can be the first line of the function (rather than tuple-making), ensuring minimum overhead. I think it makes sense to also add the tuple version to the cache here to minimize the amount of special cases (and after all it is possible to pass a single-element tuple to the __class_getitem__ function).

samuelcolvin

sorry, few more small things. Otherwise I think this looks great.

samuelcolvin · 2019-06-18T16:50:31Z

pydantic/generics.py

+
+class GenericModel(BaseModel):
+    __slots__ = ()
+    __concrete: ClassVar[bool] = False


Suggested change

__concrete: ClassVar[bool] = False

__concrete__: ClassVar[bool] = False

pydantic/generics.py

dmontagu · 2019-06-18T17:40:06Z

tests/test_generics.py

@@ -131,7 +143,7 @@ class Config:
 def test_generic_instantiation_error():
    with pytest.raises(TypeError) as exc_info:
        GenericModel()
-    assert str(exc_info.value) == 'Type GenericModel cannot be instantiated; it can be used only as a base class'
+    assert str(exc_info.value) == 'Type GenericModel cannot be used without generic parameters, e.g. GenericModel[T]'


This might be slightly confusing because you also have to inherit from Generic before this will work, but if you try (e.g. by doing GenericModel[int]) it will then tell you to inherit from typing.Generic. So I think it's okay.

dmontagu · 2019-06-18T17:42:19Z

@samuelcolvin Okay, I think I addressed the points you raised. I slightly reworked where which exceptions arise in order to make the messages a little more clear. Let me know if you think it needs more work.

dmontagu · 2019-06-18T17:44:10Z

pydantic/generics.py

+            params = (params,)
+        if any(isinstance(param, TypeVar) for param in params):  # type: ignore
+            raise TypeError(f'Type parameters should be placed on typing.Generic, not GenericModel')
+        if Generic not in cls.__bases__:


It's important to put this check for inheriting from Generic after the check for TypeVar parameterization, or the error messages end up being more confusing if you try to put type vars directly on the GenericModel (which I would expect to be a relatively common mistake).

intrepidOlivia · 2019-06-18T22:04:22Z

Just curious, since I'm interested in using this feature, do we have an ETA on when this might be merged?

drunkwcodes · 2019-06-19T01:37:03Z

I'm also curious about it but I still stand for @dmontagu's optimistic development cycle.

samuelcolvin · 2019-06-19T10:23:03Z

this is amazing, thank you very much.

samuelcolvin · 2019-06-19T10:52:48Z

@intrepidOlivia @drunkwcodes I've released a new version now with this included, should be available in pypi in a few minutes.

intrepidOlivia · 2019-06-19T16:44:24Z

Awesome! Thank you!

samuelcolvin reviewed Jun 14, 2019

View reviewed changes

dmontagu commented Jun 14, 2019

View reviewed changes

setup.py Show resolved Hide resolved

dmontagu commented Jun 15, 2019

View reviewed changes

pydantic/generics.py Outdated Show resolved Hide resolved

drunkwcodes suggested changes Jun 15, 2019

View reviewed changes

David Montague added 14 commits June 15, 2019 14:34

Added generic functionality

979656e

Skip tests in python 3.6

54e2541

double quote -> single quote

cc86e0e

Simplified, with more comprehensive tests

bbef6f6

double quote -> single quote

249d444

Remove unintentional file

ed29cae

Add caching

9e70e9e

don't cythonize generics.py

f93d12e

Make work with mypy

4044464

Remove __parameters__

7e08cb7

double quote -> single quote i'll remember one day

62f864b

More cleanup and validation

8ce4783

Removed unwanted file

eaa3737

A little more cleanup, and finish the PR

d10fb8c

dmontagu force-pushed the generic branch from 1cc98ee to aaa25b3 Compare June 15, 2019 21:37

Added note about inheritance to docs

aaa25b3

dmontagu changed the title ~~Added generic functionality~~ Add generic functionality Jun 15, 2019

dmontagu commented Jun 15, 2019

View reviewed changes

pydantic/generics.py Show resolved Hide resolved

Added error for double-parameterizing

70caaaa

David Montague added 2 commits June 15, 2019 16:12

Should build for python3.7

4d6b887

Works with both 3.6 and 3.7

ffdd6ec

David Montague added 2 commits June 17, 2019 22:30

Fixed bug with caching for single argument

d7ae62b

handle __name__ for generic models

2ba1b86

dmontagu commented Jun 18, 2019

View reviewed changes

double quote -> single quote

584de8d

dmontagu commented Jun 18, 2019

View reviewed changes

samuelcolvin reviewed Jun 18, 2019

View reviewed changes

Updated error messages

bfeb332

dmontagu commented Jun 18, 2019

View reviewed changes

Merge branch 'master' into generic

ffcf4ea

samuelcolvin merged commit b84df07 into pydantic:master Jun 19, 2019

tehrengruber mentioned this pull request Sep 1, 2020

Allow type variables in Nodes GridTools/gtc#58

Open

goodboy mentioned this pull request Feb 24, 2021

Typed messaging and validation goodboy/tractor#196

Open

3 tasks

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

Use return_schema for computed fields (pydantic#595)

027e679

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add generic functionality #595

Add generic functionality #595

dmontagu commented Jun 14, 2019 •

edited

dmontagu commented Jun 14, 2019

samuelcolvin commented Jun 14, 2019 •

edited

codecov bot commented Jun 14, 2019 •

edited

samuelcolvin left a comment

dmontagu Jun 14, 2019

drunkwcodes commented Jun 15, 2019

dmontagu commented Jun 15, 2019 •

edited

drunkwcodes commented Jun 15, 2019 •

edited

dmontagu commented Jun 15, 2019 •

edited

dmontagu commented Jun 15, 2019 •

edited

drunkwcodes commented Jun 15, 2019

drunkwcodes left a comment

drunkwcodes Jun 15, 2019 •

edited

samuelcolvin commented Jun 15, 2019

drunkwcodes commented Jun 15, 2019 •

edited

tiangolo commented Jun 15, 2019

dmontagu commented Jun 16, 2019

dmontagu commented Jun 18, 2019

dmontagu Jun 18, 2019 •

edited

tiangolo Jun 18, 2019

dmontagu Jun 18, 2019 •

edited

samuelcolvin left a comment

samuelcolvin Jun 18, 2019

dmontagu Jun 18, 2019

dmontagu commented Jun 18, 2019

dmontagu Jun 18, 2019

intrepidOlivia commented Jun 18, 2019 •

edited

drunkwcodes commented Jun 19, 2019

samuelcolvin commented Jun 19, 2019

samuelcolvin commented Jun 19, 2019

intrepidOlivia commented Jun 19, 2019

		from pydantic.class_validators import gather_validators


		class GenericModel:

	__concrete: ClassVar[bool] = False
	__concrete__: ClassVar[bool] = False

Add generic functionality #595

Add generic functionality #595

Conversation

dmontagu commented Jun 14, 2019 • edited

Change Summary

Related issue number

Checklist

dmontagu commented Jun 14, 2019

samuelcolvin commented Jun 14, 2019 • edited

codecov bot commented Jun 14, 2019 • edited

Codecov Report

samuelcolvin left a comment

Choose a reason for hiding this comment

dmontagu Jun 14, 2019

Choose a reason for hiding this comment

drunkwcodes commented Jun 15, 2019

dmontagu commented Jun 15, 2019 • edited

drunkwcodes commented Jun 15, 2019 • edited

dmontagu commented Jun 15, 2019 • edited

dmontagu commented Jun 15, 2019 • edited

drunkwcodes commented Jun 15, 2019

drunkwcodes left a comment

Choose a reason for hiding this comment

drunkwcodes Jun 15, 2019 • edited

Choose a reason for hiding this comment

samuelcolvin commented Jun 15, 2019

drunkwcodes commented Jun 15, 2019 • edited

tiangolo commented Jun 15, 2019

dmontagu commented Jun 16, 2019

dmontagu commented Jun 18, 2019

dmontagu Jun 18, 2019 • edited

Choose a reason for hiding this comment

tiangolo Jun 18, 2019

Choose a reason for hiding this comment

dmontagu Jun 18, 2019 • edited

Choose a reason for hiding this comment

samuelcolvin left a comment

Choose a reason for hiding this comment

samuelcolvin Jun 18, 2019

Choose a reason for hiding this comment

dmontagu Jun 18, 2019

Choose a reason for hiding this comment

dmontagu commented Jun 18, 2019

dmontagu Jun 18, 2019

Choose a reason for hiding this comment

intrepidOlivia commented Jun 18, 2019 • edited

drunkwcodes commented Jun 19, 2019

samuelcolvin commented Jun 19, 2019

samuelcolvin commented Jun 19, 2019

intrepidOlivia commented Jun 19, 2019

dmontagu commented Jun 14, 2019 •

edited

samuelcolvin commented Jun 14, 2019 •

edited

codecov bot commented Jun 14, 2019 •

edited

dmontagu commented Jun 15, 2019 •

edited

drunkwcodes commented Jun 15, 2019 •

edited

dmontagu commented Jun 15, 2019 •

edited

dmontagu commented Jun 15, 2019 •

edited

drunkwcodes Jun 15, 2019 •

edited

drunkwcodes commented Jun 15, 2019 •

edited

dmontagu Jun 18, 2019 •

edited

dmontagu Jun 18, 2019 •

edited

intrepidOlivia commented Jun 18, 2019 •

edited