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

Change schema frozenset #1560

Merged
merged 7 commits into from Jun 2, 2020

Conversation

wangpeibao
Copy link
Contributor

@wangpeibao wangpeibao commented May 26, 2020

Change Summary

add one type(frozenset) to schema.py/field_class_to_schema

Related issue number

fix #1557

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)

@codecov
Copy link

codecov bot commented May 26, 2020

Codecov Report

Merging #1560 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master     #1560   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         3793      3796    +3     
  Branches       752       752           
=========================================
+ Hits          3793      3796    +3     
Impacted Files Coverage Δ
pydantic/schema.py 100.00% <ø> (ø)
pydantic/validators.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2eb62a3...6e53648. Read the comment docs.

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

please add tests.

Comment on lines 1 to 2
change schema.py/field_class_to_schema
add frozenset type to the tuple
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
change schema.py/field_class_to_schema
add frozenset type to the tuple
change `schema.field_class_to_schema` to support `frozenset` in schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I do not know how to add tests.

Copy link
Member

Choose a reason for hiding this comment

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

you need to commit this suggested change.

@samuelcolvin
Copy link
Member

Look at teat/test_schema.py and add another test, it's not that hard.

changes/index-1.html Outdated Show resolved Hide resolved
Comment on lines 1 to 2
change schema.py/field_class_to_schema
add frozenset type to the tuple
Copy link
Member

Choose a reason for hiding this comment

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

you need to commit this suggested change.

@@ -1837,7 +1837,10 @@ class Model(BaseModel):
def test_frozen_set():
class Model(BaseModel):
a: FrozenSet[int] = frozenset({1, 2, 3})
b: FrozenSet = frozenset({1, 2, 3})
c: frozenset = frozenset({1, 2, 3})
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
c: frozenset = frozenset({1, 2, 3})
c: frozenset = frozenset({1, 2, 3})
d: frozenset = ...

Just to check the case when there's no default (obviously you'll need to change the check below too.

@samuelcolvin samuelcolvin merged commit 5e82689 into pydantic:master Jun 2, 2020
@samuelcolvin
Copy link
Member

thank you so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add frozenset to schema.py/field_class_to_schema
2 participants