JSON Schema update/refactor/augment, to conform to spec #308

tiangolo · 2018-11-17T17:19:43Z

Update/augment JSON Schema implementation to conform to JSON Schema spec

Note

I renamed BaseModel.schema_json to BaseModel.schema_str, as it was a bit confusing for me the first time I tried Pydantic. As this is a change mostly related to "taste", let me know if you want me to revert it. Or maybe rename it to something like schema_dump.

Description

Generated JSON schema from a model is now fully conforming to the JSON Schema spec
Added a lot of logic to declare special and custom types, including:
- Date and time types
- Pydantic's types, including constraints, as max_length, regex, etc.
Types are mapped to match as possible the specs for JSON Schema Core, JSON Schema Validation and OpenAPI. E.g. EmailStr generates a "type": "string", "format": "email", equivalent for datetime. bytes get a "format": "binary", although there is no equivalent in JSON Schema, that is the format defined in OpenAPI, etc.
Refactor all schema handling out of the main classes
Models are defined in a separate definitions key, this allows better re-use of the definitions
- Only applies for models and when not overwritten (with default values, custom title, etc)
Models defined in definitions key are checked and processed to have unique names, so modulea.Model will be named Model if there is no competing class, but if there is a moduleb.Model, both models will have corresponding unique names (modulea__Model and moduleb__Model)
utility function to generate a single JSON Schema with definitions from several models that can be re-utilized (e.g. in OpenAPI components).
Update all (or most of) the tests for JSON Schema
Add many new tests to check all the new functionality
I plan on reviewing the tests again and augment them if necessary once we check that these changes seem acceptable. After that, I'll update the docs for Schema generation.

Related issue number

#291

Performance Changes

pydantic cares about performance, if there's any risk performance changed on this PR,
please run make benchmark-pydantic before and after the change:

before: ?
after: ?

JSON Schemas are generaly only run once, and are also cached. As they are more like a "colateral" feature that is not involved in the normal worflow of Pydantic, I don't think performance could be affected or relevant for this PR.

Checklist

Unit tests for the changes exist
Tests pass on CI and coverage remains at 100%: Test coverage for the new module at 100%. Added tests for utils to cover stuff that was covered with the previous implementation.
Documentation reflects the changes: Docs for Schema JSON were updated accordingly, as thoroughly and clear as possible.
[ 🤔 ] No performance deterioration (if applicable): I think it doesn't apply to this PR.
HISTORY.rst has been updated: 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>
- if you're not a regular contributer please include your github username @whatever

samuelcolvin · 2018-11-17T17:48:40Z

Conflicts
not sure why tests aren't running, commit again and see if it's fixed
Do we really need the new test module? I doubt it.
Coverage should be 100% as it is on Master.

tiangolo · 2018-11-17T18:40:36Z

Rebased and pushed.
The tests are passing locally. Let me fix the remote errors that seem to be flake, I was not running it locally, I was just running make test.
The new test module are just dummy models. I don't like having an extra module with sub-modules just for testing either. But that is the way we can test having two models named Model and check that the JSON Schema generation creates unique names for both of them based on the module path. And that it doesn't duplicate an imported model that was already created somewhere else.
I'll work on the coverage.

samuelcolvin · 2018-11-17T18:42:17Z

Run just make to run both tests and linting.

codecov · 2018-11-18T09:52:35Z

Codecov Report

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

@@          Coverage Diff           @@
##           master   #308    +/-   ##
======================================
  Coverage     100%   100%            
======================================
  Files          12     13     +1     
  Lines        1532   1703   +171     
  Branches      285    321    +36     
======================================
+ Hits         1532   1703   +171

samuelcolvin

Looking good initially, but quite a lot to change.

I'll probably have some more stuff I want fixing once this is resolved but hard to see without fixing my initial concerns.

If there's loads more to do I might submit a PR against this PR to save you some time.

pydantic/main.py

pydantic/schema.py

samuelcolvin · 2018-11-18T15:01:06Z

pydantic/schema.py

+
+
+def get_flat_models_from_model(model: Type['main.BaseModel']) -> Set[Type['main.BaseModel']]:
+    flat_models: Set[Type[main.BaseModel]] = set()


we generally don't both with type annotations inside methods.

Done. I removed the others too.

samuelcolvin · 2018-11-18T15:03:28Z

pydantic/schema.py

+from .utils import clean_docstring
+
+
+def get_flat_models_from_model(model: Type['main.BaseModel']) -> Set[Type['main.BaseModel']]:


is main.* enough for mypy or ids to work out what BaseModel is? Surely pydantic.BaseModel or pydantic.main.BaseModel is required?

also if we can import BaseModel, just use that rather than a string.

I can't import it directly as these functions are imported and used inside of BaseModel. I had to use that "trick" from PEP 484.

If you want I can change it to:

def get_flat_models_from_model(model: Type) -> Set[Type]:

But for it to work it needs the models to be Pydantic, i.e, to have __fields__. That's why I annotated it like that.

Let me know what do you prefer.

you already import main above, so therefore surely you can use main.BaseModel or better import BaseModel from main and use it here.

It can't be done with circular imports. main.BaseModel already depends on these methods, so BaseModel is not yet defined by the moment these methods are defined, as they are imported and used by main.BaseModel first, check the errors in CI: https://travis-ci.org/samuelcolvin/pydantic/jobs/457945798

From PEP 484: Forward references:

When a type hint contains names that have not been defined yet, that definition may be expressed as a string literal, to be resolved later.

A situation where this occurs commonly is the definition of a container class, where the class being defined occurs in the signature of some of the methods. For example, the following code (the start of a simple binary tree implementation) does not work:

class Tree: def __init__(self, left: Tree, right: Tree): self.left = left self.right = right

To address this, we write:

class Tree: def __init__(self, left: 'Tree', right: 'Tree'): self.left = left self.right = right

Later in the same section:

[...] The solution is to switch to module-only imports and reference the models by their module.class name:

# File models/a.py from models import b class A(Model): def foo(self, b: 'b.B'): ...

samuelcolvin · 2018-11-18T15:04:00Z

pydantic/schema.py

+    return flat_models
+
+
+def get_flat_models_from_sub_fields(fields) -> Set[Type['main.BaseModel']]:


rename to get_flat_models_from_fields and reuse in get_flat_models_from_model

samuelcolvin · 2018-11-18T15:22:15Z

pydantic/schema.py

+        if field.type_.le is not None:
+            f_schema.update({'maximum': field.type_.le})
+    # Sub-classes of str must go before str
+    if issubclass(field.type_, EmailStr):


again some kind of iterable and loop would be much better here.

samuelcolvin · 2018-11-18T15:23:32Z

pydantic/schema.py

+from .types import DSN, UUID1, UUID3, UUID4, UUID5, DirectoryPath, EmailStr, FilePath, Json, NameEmail, UrlStr
+from .utils import clean_docstring
+
+


best to put all these "helper" methods at the bottom of the file and the "primary" methods (those people might want to read first) at the top.

Also, you should probably implement __all__ here.

samuelcolvin · 2018-11-18T15:25:09Z

tests/schema_test_package/modulea/modela.py

@@ -0,0 +1,5 @@
+from pydantic import BaseModel


please remove all this and put it into the tests.

If you really need some python modules to test use tmpdir and write the files required within the test.

samuelcolvin · 2018-11-18T15:27:09Z

tests/test_schema.py

+    }
+
+
+def test_date_types():


all these multiple type tests would be better rewritten as parameterised tests - if one case here fails it'll be a lot easier to work out which one an fix if they're 4 separate tests.

samuelcolvin · 2018-11-18T15:30:07Z

tests/test_schema.py

+
+    class Pizza(BaseModel):
+        name: str
+        ingredients: List[Ingredient]


do we really need all these models to test the functionality? Unit tests should be as simple as possible while testing the desired behaviour.

I removed one of the submodels. But having at least two models with submodels would verify that the submodels are included not only for the first model in the list. Let me know if you still want me to reduce more.

HISTORY.rst

samuelcolvin · 2018-11-18T15:35:18Z

Byt the way,

You can run make format to call black and isort and fix all formatting, best to do before every commit.
you can rerun tests from within travis, also with empty commits useing --allow-empty

samuelcolvin · 2018-11-18T17:15:06Z

@tiangolo so far I have 51 emails from you (via github) in the last few days.

Please get the PR to the point where you'd like me to review it and let me know.

I don't need replies to every single code comment and every line changed doesn't need its own commit.

tiangolo · 2018-11-18T19:12:28Z

You can run make format to call black and isort and fix all formatting, best to do before every commit.

you can rerun tests from within travis, also with empty commits useing --allow-empty

Thanks for the tips.

@tiangolo so far I have 51 emails from you (via github) in the last few days.
Please get the PR to the point where you'd like me to review it and let me know.
I don't need replies to every single code comment and every line changed doesn't need its own commit.

Sorry for that. Now I tried to avoid pushing before finishing with the rest of the review.

I see now that there are new conflicts with HISTORY.rst and pydantic/fields.py. Should I rebase or merge master?

Edit: I see you requested a rebase in another PR so I did that.

including datetime and all supported Pydantic.types

and update/simplify internal Schema methods

and generation of a single JSON Schema with definitions from multiple (unrelated) models

…ytest

…ick in Travis for non owners

tiangolo · 2018-11-18T19:52:32Z

OK, that's it for now.

Python 3.7-dev keeps failing with some random race condition or something that comes from time to time. I could try re-pushing empty commits to try and make it run again, but I guess I that would trigger more emails. And we non-owners don't see the "Re-start build" button in Travis CI.

I think I already implemented all your requested changes.

If something else is missing or should be changed, let me know. I'll (hopefully) check it in the next days. But it's almost midnight here and I can't "extend" the weekend more. ...and my wife is not loving the project right now 😂

tiangolo · 2018-11-21T14:52:21Z

@samuelcolvin let me know if there's anything missing.

samuelcolvin · 2018-11-22T14:51:07Z

frustratingly although it should let me, github isn't letting me push to your branch and therefore make changes to the PR.

Instead, I've submitted a PR tiangolo#1 to your repo on this branch. If you could review that, ask any questions and merge it.

If that's all ok, I think we'll be ok to merge this.

tiangolo · 2018-11-22T15:21:01Z

frustratingly although it should let me, github isn't letting me push to your branch and therefore make changes to the PR.

Strange...

Instead, I've submitted a PR tiangolo#1 to your repo on this branch. If you could review that, ask any questions and merge it.

Awesome! I'll check it right away.

tiangolo · 2018-11-22T15:58:28Z

I changed the links to the specs. If you want, check them.

samuelcolvin · 2018-11-22T15:59:31Z

I'm sure they're right, are you happy for me to merge this?

tiangolo · 2018-11-22T15:59:49Z

Yes! 🌮

samuelcolvin · 2018-11-22T16:00:39Z

I'll release in a couple of days. If anyone has any further feedback on this, let us know here.

tiangolo · 2018-11-22T16:03:57Z

Awesome, thanks. 🎉

I'll start working on a PR to integrate validation and schema annotations passed to the Schema class.

* uprev-deps * cargo update

tiangolo force-pushed the json-schema-flat branch from a7669b8 to 8c555ba Compare November 17, 2018 18:34

tiangolo changed the title ~~[WIP] JSON Schema update/refactor/augment, to conform to spec~~ JSON Schema update/refactor/augment, to conform to spec Nov 18, 2018

samuelcolvin requested changes Nov 18, 2018

View reviewed changes

samuelcolvin reviewed Nov 18, 2018

View reviewed changes

HISTORY.rst Outdated Show resolved Hide resolved

samuelcolvin force-pushed the master branch from 2e10b34 to 9ff946d Compare November 18, 2018 16:11

tiangolo added 18 commits November 18, 2018 23:17

Update schema tests to conform to JSON Schema spec

9e7c55f

Add JSON Schema tests for all supported types

e3196f6

including datetime and all supported Pydantic.types

Add JSON Schema conforming schema sub module

f905f83

Update BaseModel to use schema module for JSON Schema generation

a702d6d

and update/simplify internal Schema methods

Remove Schema code from Field class, replaced with JSON Schema module

bc73427

Add submodules to test model name generation for JSON Schemas

1312ca4

Refactor/rewrite schema module to generate definions and refs

18a2b25

Update and augment JSON Schema tests to include definitions and refs

c88a760

and generation of a single JSON Schema with definitions from multiple (unrelated) models

Add ref_prefix functionality to JSON Schema generation functions

b24bb29

Test custom ref_prefix in JSON Schema generation

580cc28

Remove un-used BaseModel method, now refactored to schema module

0d18d7d

Update formating of test_schema

1ec4aba

Fix long lines in test_schema

115883b

Fix imported but unused in fields

343b504

Fix imported but unused in main.py

0391d62

Ignore imported but unused for testing modulec

fcf34a4

Refactor schema module for complexity

5c642cb

Add conflicting name model to raise coverage

56b377b

tiangolo added 4 commits November 18, 2018 23:22

Implement __all__, move and order parts of schema

0a24236

Remove schema testing sub-package code as requested

033670b

Generate schema testing subpackage in code

e22a899

Update schema tests with several related fields to use parametrized p…

a73818e

…ytest

tiangolo force-pushed the json-schema-flat branch from 517bad9 to a73818e Compare November 18, 2018 19:24

tiangolo added 3 commits November 18, 2018 23:30

Fix formatting and imports I missed after rebase

d08eafb

Fix new formatting errors from CI

1870bd2

Re-trigger Travis CI, Python 3.7-dev random error again, no re-run cl…

66d9f23

…ick in Travis for non owners

tiangolo added 2 commits November 21, 2018 17:41

Trigger annotation error with non-forward references

b11196d

Add docstrings for submodel schema

b714acc

samuelcolvin added 2 commits November 22, 2018 14:23

tweaks and rewrite schema mapping table in python

1604f72

support complex defaults

1aef328

samuelcolvin and others added 3 commits November 22, 2018 15:49

use str not int as dict keys

3ce82a6

Fix links to JSON Schema and OpenAPI

229dfaa

Merge branch 'samuelcolvin-json-schema-flat' into json-schema-flat

aa3854b

samuelcolvin approved these changes Nov 22, 2018

View reviewed changes

samuelcolvin merged commit 94706bc into pydantic:master Nov 22, 2018

tiangolo deleted the json-schema-flat branch November 22, 2018 16:04

samuelcolvin mentioned this pull request Jan 10, 2019

Refactored extra types to use a single enum #352

Merged

4 tasks

kryft mentioned this pull request Oct 30, 2019

Customizing both decoding and encoding of a type #951

Closed

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

uprev-deps (pydantic#308)

8f2daf6

* uprev-deps * cargo update



		def get_flat_models_from_model(model: Type['main.BaseModel']) -> Set[Type['main.BaseModel']]:
		flat_models: Set[Type[main.BaseModel]] = set()

		from .utils import clean_docstring


		def get_flat_models_from_model(model: Type['main.BaseModel']) -> Set[Type['main.BaseModel']]:

		return flat_models


		def get_flat_models_from_sub_fields(fields) -> Set[Type['main.BaseModel']]:

		from .types import DSN, UUID1, UUID3, UUID4, UUID5, DirectoryPath, EmailStr, FilePath, Json, NameEmail, UrlStr
		from .utils import clean_docstring

Uh oh!

JSON Schema update/refactor/augment, to conform to spec #308

JSON Schema update/refactor/augment, to conform to spec #308

Uh oh!

Conversation

tiangolo commented Nov 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Update/augment JSON Schema implementation to conform to JSON Schema spec

Note

Description

Related issue number

Performance Changes

Checklist

Uh oh!

samuelcolvin commented Nov 17, 2018

Uh oh!

tiangolo commented Nov 17, 2018

Uh oh!

samuelcolvin commented Nov 17, 2018

Uh oh!

codecov bot commented Nov 18, 2018 • 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samuelcolvin commented Nov 18, 2018

Uh oh!

samuelcolvin commented Nov 18, 2018

Uh oh!

tiangolo commented Nov 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tiangolo commented Nov 18, 2018

Uh oh!

tiangolo commented Nov 21, 2018

Uh oh!

samuelcolvin commented Nov 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

tiangolo commented Nov 17, 2018 •

edited

Loading

codecov bot commented Nov 18, 2018 •

edited

Loading

tiangolo commented Nov 18, 2018 •

edited

Loading

samuelcolvin commented Nov 22, 2018 •

edited

Loading