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

Fix schema for set and frozenset with default value #4155

Merged
merged 4 commits into from Aug 8, 2022
Merged

Fix schema for set and frozenset with default value #4155

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a462b7
Fix schema for Set with default value
aminalaee Jun 12, 2022
ffc8704
add changelog
aminalaee Jun 12, 2022
cb65957
Merge branch 'master' of github.com:samuelcolvin/pydantic into fix-se…
aminalaee Jul 14, 2022
d4412be
Add frozenset to schema mapping docs
aminalaee Jul 14, 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/4155-aminalaee.md
View file
Open in desktop
@@ -0,0 +1 @@
Fix JSON schema for `set` and `frozenset` when they include default values.
4 changes: 2 additions & 2 deletions pydantic/schema.py
View file
Open in desktop
Expand Up @@ -77,7 +77,7 @@
is_none_type,
is_union,
)
from .utils import ROOT_KEY, get_model, lenient_issubclass, sequence_like
from .utils import ROOT_KEY, get_model, lenient_issubclass

if TYPE_CHECKING:
from .dataclasses import Dataclass
Expand Down Expand Up @@ -984,7 +984,7 @@ def encode_default(dft: Any) -> Any:
return dft.value
elif isinstance(dft, (int, float, str)):
return dft
elif sequence_like(dft):
elif isinstance(dft, (list, tuple)):
Copy link

Choose a reason for hiding this comment

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

I'm really not into the code base - so this is just a non-constructive critisism, but this change would narrow the scope compared to the original sequence_like check. What happens when these instances (set, frozenset, ...) are passed in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm also very new to the codebase, but I think the sequence_like usage is not very accurate here.

If you look further, after these conditions it will try to use the pydantic_encoder for all unhandled types which in this case is set and frozenset.
And also there are tests for all cases and new ones for set and frozenset, so I think this is not narrowing down any type, just setting up different encoders for these types.

Copy link

Choose a reason for hiding this comment

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

sounds reasonable! now we only need @samuelcolvin or @PrettyWood to approve :-)

t = dft.__class__
seq_args = (encode_default(v) for v in dft)
return t(*seq_args) if is_namedtuple(t) else t(seq_args)
Expand Down
8 changes: 5 additions & 3 deletions tests/test_schema.py
View file
Open in desktop
Expand Up @@ -530,13 +530,15 @@ def test_set():
class Model(BaseModel):
a: Set[int]
b: set
c: set = {1}

assert Model.schema() == {
'title': 'Model',
'type': 'object',
'properties': {
'a': {'title': 'A', 'type': 'array', 'uniqueItems': True, 'items': {'type': 'integer'}},
'b': {'title': 'B', 'type': 'array', 'items': {}, 'uniqueItems': True},
'c': {'title': 'C', 'type': 'array', 'items': {}, 'default': [1], 'uniqueItems': True},
},
'required': ['a', 'b'],
}
Expand Down Expand Up @@ -2187,13 +2189,13 @@ class Model(BaseModel):
'properties': {
'a': {
'title': 'A',
'default': frozenset({1, 2, 3}),
'default': [1, 2, 3],
'type': 'array',
'items': {'type': 'integer'},
'uniqueItems': True,
},
'b': {'title': 'B', 'default': frozenset({1, 2, 3}), 'type': 'array', 'items': {}, 'uniqueItems': True},
'c': {'title': 'C', 'default': frozenset({1, 2, 3}), 'type': 'array', 'items': {}, 'uniqueItems': True},
'b': {'title': 'B', 'default': [1, 2, 3], 'type': 'array', 'items': {}, 'uniqueItems': True},
'c': {'title': 'C', 'default': [1, 2, 3], 'type': 'array', 'items': {}, 'uniqueItems': True},
'd': {'title': 'D', 'type': 'array', 'items': {}, 'uniqueItems': True},
},
'required': ['d'],
Expand Down