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

support custom root types #628

Merged
merged 12 commits into from Jul 6, 2019

Conversation

Projects
None yet
2 participants
@koxudaxi
Copy link
Contributor

commented Jun 29, 2019

Change Summary

support custom root types that support a model that does not have any field exclude __root__.

example:

from typing import List
import json
from pydantic import BaseModel
from pydantic.schema import schema

class Pets(BaseModel):
    __root__: List[str]

print(Pets(__root__=['dog', 'cat']))
# > Pets __root__=['dog', 'cat']

print(Pets(*['dog', 'cat']))
# > Pets __root__=['dog', 'cat']

print(Pets(*['dog', 'cat']).schema())
# > {'title': 'Pets', 'type': 'array', 'items': {'type': 'string'}}

pets_schema = schema([Pets])
print(json.dumps(pets_schema, indent=2))

# {
#  "definitions": {
#    "Pets": {
#      "title": "Pets",
#      "type": "array",
#      ...

Related issue number

#507

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
    • this pull request number #628
    • include github username @koxudaxi

koxudaxi added some commits Jun 29, 2019

@codecov

This comment has been minimized.

Copy link

commented Jun 29, 2019

Codecov Report

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

@@          Coverage Diff          @@
##           master   #628   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines        2634   2651   +17     
  Branches      518    524    +6     
=====================================
+ Hits         2634   2651   +17
@samuelcolvin
Copy link
Owner

left a comment

Otherwise the idea looks good, I'll review more once it's complete.

@@ -466,6 +466,18 @@ Outputs:
.. literalinclude:: examples/schema3.json


Pydantic models can be set custom root types to support a model that does not have any field exclude ``__root__``.

This comment has been minimized.

Copy link
@samuelcolvin

samuelcolvin Jul 1, 2019

Owner

Can you make this a separate section of the docs. This is not just about schema generation.

This comment has been minimized.

Copy link
@koxudaxi

koxudaxi Jul 1, 2019

Author Contributor

I see. I have moved the content to around types sections.

@@ -186,7 +186,7 @@ def __new__(mcs, name, bases, namespace):
for ann_name, ann_type in annotations.items():
if is_classvar(ann_type):
class_vars.add(ann_name)
elif not ann_name.startswith('_') and ann_name not in namespace:
elif (not ann_name.startswith('_') or '__root__' == ann_name) and ann_name not in namespace:

This comment has been minimized.

Copy link
@samuelcolvin

samuelcolvin Jul 1, 2019

Owner

we do this at least three times, can we have a function for it.

This comment has been minimized.

Copy link
@koxudaxi

koxudaxi Jul 1, 2019

Author Contributor

I have created is_valid_field function for checking a field name.

_json_encoder: Callable[[Any], Any] = lambda x: x
_schema_cache: 'DictAny' = {}

Config = BaseConfig
__slots__ = ('__values__', '__fields_set__')

def __init__(self, **data: Any) -> None:
def __init__(self, *__root__: Any, **data: Any) -> None:

This comment has been minimized.

Copy link
@samuelcolvin

samuelcolvin Jul 1, 2019

Owner

Humm, I've very unwilling to change the signature of __init__.

Would it be possible to accept __root__ directly to parse_obj only?

This comment has been minimized.

Copy link
@koxudaxi

koxudaxi Jul 1, 2019

Author Contributor

I have reverted the __init__ method to master.
And I add a __root__ argument in parse_obj.
However, We can pass __root__ object to __init__ as a keyword argument.

Model(__root__='a')
# or
Model.parse_obj(__root__='a')
remove the keyword argument of "__root__" in __init__
add a keyword argument of "__root__" in parse_obj
fix documents
create a method for cheking valid field name
@samuelcolvin
Copy link
Owner

left a comment

otherwise looking good.

Show resolved Hide resolved docs/index.rst Outdated
Show resolved Hide resolved docs/index.rst Outdated
Show resolved Hide resolved docs/index.rst Outdated
@@ -233,6 +241,7 @@ class BaseModel(metaclass=MetaModel):
__fields__: Dict[str, Field] = {}
__validators__: Dict[str, AnyCallable] = {}
__config__: Type[BaseConfig] = BaseConfig
__root__: List[Any] = []

This comment has been minimized.

Copy link
@samuelcolvin

samuelcolvin Jul 2, 2019

Owner

I thought __root__ doesn't have to be a list?

This comment has been minimized.

Copy link
@koxudaxi

koxudaxi Jul 2, 2019

Author Contributor

Sorry, it's my mistake. I have changed List[Any] to Any.

@@ -211,6 +217,8 @@ def __new__(mcs, name, bases, namespace):
config=config,
)

if '__root__' in fields and len(fields) > 1:
raise ValueError('__root__ cannot be mixed with other fields')

This comment has been minimized.

Copy link
@samuelcolvin

samuelcolvin Jul 2, 2019

Owner

add _custom_root_type: bool to new_namespace below.

This comment has been minimized.

Copy link
@koxudaxi

koxudaxi Jul 2, 2019

Author Contributor

I have added it.

@@ -324,9 +334,17 @@ def json(
)

@classmethod
def parse_obj(cls: Type['Model'], obj: Mapping[Any, Any]) -> 'Model':
def parse_obj(
cls: Type['Model'], obj: Optional[Mapping[Any, Any]] = None, __root__: Optional[Any] = None

This comment has been minimized.

Copy link
@samuelcolvin

samuelcolvin Jul 2, 2019

Owner

simplify this to

def parse_obj(cls: Type['Model'], obj: Any) -> 'Model'

This comment has been minimized.

Copy link
@koxudaxi

koxudaxi Jul 2, 2019

Author Contributor

ok, I just changed it.

if not isinstance(obj, dict):
try:
if obj is None:

This comment has been minimized.

Copy link
@samuelcolvin

samuelcolvin Jul 2, 2019

Owner

change this to:

if not instanstance(obj, dict):
    if cls._custom_root_type:
        obj = {'__root__': obj}
    else:
        ...

This comment has been minimized.

Copy link
@samuelcolvin

samuelcolvin Jul 2, 2019

Owner

I think we should also raise an error if the type of __root__ is a dict.

This comment has been minimized.

Copy link
@koxudaxi

koxudaxi Jul 2, 2019

Author Contributor

I have changed the condition and added to raise an exception.

class MyModel(BaseModel):
__root__: str

m = MyModel(__root__='a')

This comment has been minimized.

Copy link
@samuelcolvin

samuelcolvin Jul 2, 2019

Owner

please test with parse_obj too.

This comment has been minimized.

Copy link
@koxudaxi

koxudaxi Jul 2, 2019

Author Contributor

I had made tests for parse_obj in tests/tset_parse.py
Would you please check it?
https://github.com/samuelcolvin/pydantic/pull/628/files#diff-7b34c8bd665994bbdcf056c03622d4a8R45

koxudaxi and others added some commits Jul 2, 2019

Update docs/index.rst
Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>
Apply suggestions from code review
Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>
@samuelcolvin
Copy link
Owner

left a comment

sorry, reviewed this days ago and somehow forgot to press submit.

Show resolved Hide resolved docs/index.rst Outdated
def parse_obj(cls: Type['Model'], obj: Any) -> 'Model':
if isinstance(obj, dict):
if cls._custom_root_type:
raise TypeError('custom root type cannot allow dict')

This comment has been minimized.

Copy link
@samuelcolvin

samuelcolvin Jul 5, 2019

Owner

this test should be moved to model creation, not validation.

This comment has been minimized.

Copy link
@koxudaxi

koxudaxi Jul 6, 2019

Author Contributor

OK, I have moved it to __new__ in MetaModel.
Also, I change the validation type from dict to Mapping. If it's wrong, then please tell me correct.

koxudaxi and others added some commits Jul 6, 2019

Update docs/index.rst
Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>
@koxudaxi

This comment has been minimized.

Copy link
Contributor Author

commented Jul 6, 2019

sorry, reviewed this days ago and somehow forgot to press submit.

No problem, Thank you for reviewing my code.

@samuelcolvin samuelcolvin merged commit e4b285a into samuelcolvin:master Jul 6, 2019

2 of 7 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
deploy/netlify Deploy preview processing.
Details
Header rules No header rules processed
Details
Pages changed 2 new files uploaded
Details
Redirect rules No redirect rules processed
Details
Mixed content No mixed content detected
Details
samuelcolvin.pydantic Build #20190706.5 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.