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.

Sign up for GitHub

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

Jump to bottom

[1.10.x] Properly encode model and dataclass default for schema #4781

Merged
merged 7 commits into from Nov 29, 2022
Merged

[1.10.x] Properly encode model and dataclass default for schema #4781

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c7b1095
Properly encode model and dataclass default for schema
Bobronium Nov 24, 2022
f9349ba
Wrap type annotation in quotes
Bobronium Nov 24, 2022
3dd5951
Fix compatibility
Bobronium Nov 24, 2022
74ceaaf
Add changes file
Bobronium Nov 24, 2022
87d1a38
Remove unnecessary import
Bobronium Nov 29, 2022
6a10b6b
Add a blank line back
Bobronium Nov 29, 2022
8a41c4a
Reorder conditions
Bobronium Nov 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/4781-Bobronium.md
View file
Open in desktop
@@ -0,0 +1 @@
Fix `schema` and `schema_json` on models where a model instance is a one of default values.
10 changes: 8 additions & 2 deletions pydantic/schema.py
View file
Open in desktop
@@ -1,6 +1,7 @@
import re
import warnings
from collections import defaultdict
from dataclasses import is_dataclass
from datetime import date, datetime, time, timedelta
from decimal import Decimal
from enum import Enum
Expand Down Expand Up @@ -971,15 +972,20 @@ def multitypes_literal_field_for_schema(values: Tuple[Any, ...], field: ModelFie


def encode_default(dft: Any) -> Any:
if isinstance(dft, Enum):
from .main import BaseModel

if isinstance(dft, BaseModel) or is_dataclass(dft):
dft = cast('dict[str, Any]', pydantic_encoder(dft))
elif isinstance(dft, Enum):
return dft.value
elif isinstance(dft, (int, float, str)):
return dft
elif isinstance(dft, (list, tuple)):
t = dft.__class__
seq_args = (encode_default(v) for v in dft)
return t(*seq_args) if is_namedtuple(t) else t(seq_args)
elif isinstance(dft, dict):

if isinstance(dft, dict):
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this change here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Notice the change above: we transform models and dataclasses to dict in first if and need this to be a separate one to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any ideas how to make this more explicit and obvious, btw?

Copy link
Member

Choose a reason for hiding this comment

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

maybe just a comment with the above explanation.

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 add the first if as a separate if and move the isinstance(dft, dict) check to the top.

Here is my suggestion(I haven't tested it locally):

def encode_default(dft: Any) -> Any:
    from .main import BaseModel

    if isinstance(dft, BaseModel) or is_dataclass(dft):
        dft = cast('dict[str, Any]', pydantic_encoder(dft))

    if isinstance(dft, dict):
        return {encode_default(k): encode_default(v) for k, v in dft.items()}
    elif isinstance(dft, Enum):
        return dft.value
    elif isinstance(dft, (int, float, str)):
        return dft
    elif isinstance(dft, (list, tuple)):
        t = dft.__class__
        seq_args = (encode_default(v) for v in dft)
        return t(*seq_args) if is_namedtuple(t) else t(seq_args)
    elif dft is None:
        return None
    else:
        return pydantic_encoder(dft)

Copy link
Contributor Author

@Bobronium Bobronium Nov 29, 2022 •
edited

Choose a reason for hiding this comment

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

Happy to accept suggestions on phrasing as well :)

Previous suggestion
Suggested change
if isinstance(dft, dict):
# we can end up here either from first condition or if none of above conditions were met
if isinstance(dft, dict):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for suggestion, @hramezani, I like your approach! I think I was hesitant on reordering at first, but it seems like the best move here, indeed.

return {encode_default(k): encode_default(v) for k, v in dft.items()}
elif dft is None:
return None
Expand Down
32 changes: 32 additions & 0 deletions tests/test_schema.py
View file
Open in desktop
Expand Up @@ -1523,6 +1523,38 @@ class UserModel(BaseModel):
}


def test_model_default():
"""Make sure inner model types are encoded properly"""

class Inner(BaseModel):
a: Dict[Path, str] = {Path(): ''}

class Outer(BaseModel):
inner: Inner = Inner()

assert Outer.schema() == {
'definitions': {
'Inner': {
'properties': {
'a': {
'additionalProperties': {'type': 'string'},
'default': {'.': ''},
'title': 'A',
'type': 'object',
}
},
'title': 'Inner',
'type': 'object',
}
},
'properties': {
'inner': {'allOf': [{'$ref': '#/definitions/Inner'}], 'default': {'a': {'.': ''}}, 'title': 'Inner'}
},
'title': 'Outer',
'type': 'object',
}


@pytest.mark.parametrize(
'kwargs,type_,expected_extra',
[
Expand Down