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
Generate a hash function when frozen
is True
#1881
Changes from 6 commits
7df3d9d
75d5227
b4befc5
12cff48
0d6eaec
7397b67
4815e4c
7c3f4de
a6943a8
3e98d57
ad82bf0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Add a new `frozen` boolean parameter to `Config` (default: `False`). | ||
Setting `frozen=True` does everything that `allow_mutation=False` does, and also generates a `__hash__()` method for the model. This makes instances of the model potentially hashable if all the attributes are hashable. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -202,3 +202,37 @@ class AddProject: | |
|
||
|
||
p = AddProject(name='x', slug='y', description='z') | ||
|
||
|
||
# Same as Model, but with frozen = True | ||
class FrozenModel(BaseModel): | ||
x: int | ||
y: str | ||
|
||
def method(self) -> None: | ||
pass | ||
|
||
class Config: | ||
alias_generator = None | ||
frozen = True | ||
extra = Extra.forbid | ||
|
||
def config_method(self) -> None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you need this? |
||
... | ||
|
||
|
||
frozenmodel = FrozenModel(x=1, y='y', z='z') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this isn't testing frozen behaviour I think, again can be removed. |
||
frozenmodel = FrozenModel(x=1) | ||
frozenmodel.y = 'a' | ||
FrozenModel.from_orm({}) | ||
FrozenModel.from_orm({}) # type: ignore[pydantic-orm] # noqa F821 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again, are these lines testing frozen behaviour? |
||
|
||
|
||
class InheritingModel2(FrozenModel): | ||
class Config: | ||
frozen = False | ||
|
||
|
||
inheriting2 = InheritingModel2(x='1', y='1') | ||
Settings(x='1') | ||
FrozenModel(x='1', y='2') |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -139,3 +139,22 @@ class Model(BaseModel): | |
|
||
dynamic_model = DynamicModel(x=1, y='y') | ||
dynamic_model.x = 2 | ||
|
||
|
||
class FrozenModel(BaseModel): | ||
x: int | ||
|
||
class Config: | ||
frozen = True | ||
|
||
|
||
class NotFrozenModel(FrozenModel): | ||
a = 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add a type here to avoid extra errors in |
||
|
||
class Config: | ||
frozen = False | ||
orm_mode = True | ||
|
||
|
||
NotFrozenModel(x=1).x = 2 | ||
NotFrozenModel.from_orm(model) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
29: error: Unexpected keyword argument "z" for "Model" [call-arg] | ||
64: error: Untyped fields disallowed [pydantic-field] | ||
79: error: Argument "x" to "OverrideModel" has incompatible type "float"; expected "int" [arg-type] | ||
125: error: Untyped fields disallowed [pydantic-field] | ||
125: error: Untyped fields disallowed [pydantic-field] | ||
152: error: Untyped fields disallowed [pydantic-field] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -351,39 +351,96 @@ class Model(BaseModel): | |
assert exc_info.value.errors() == [{'loc': ('a',), 'msg': 'field required', 'type': 'value_error.missing'}] | ||
|
||
|
||
def test_not_immutability(): | ||
@pytest.mark.parametrize('allow_mutation_, frozen_', [(False, False), (True, False), (False, True), (True, True)]) | ||
def test_immutability(allow_mutation_, frozen_): | ||
class TestModel(BaseModel): | ||
a: int = 10 | ||
|
||
class Config: | ||
allow_mutation = True | ||
allow_mutation = allow_mutation_ | ||
extra = Extra.forbid | ||
frozen = frozen_ | ||
|
||
m = TestModel() | ||
assert m.a == 10 | ||
m.a = 11 | ||
assert m.a == 11 | ||
with pytest.raises(ValueError) as exc_info: | ||
m.b = 11 | ||
assert '"TestModel" object has no field "b"' in exc_info.value.args[0] | ||
|
||
immutable = allow_mutation_ is False or frozen_ is True | ||
|
||
if immutable: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rather than this if clause, better to setup two separate tests. |
||
assert m.a == 10 | ||
with pytest.raises(TypeError) as exc_info: | ||
m.a = 11 | ||
assert '"TestModel" is immutable and does not support item assignment' in exc_info.value.args[0] | ||
with pytest.raises(ValueError) as exc_info: | ||
m.b = 11 | ||
assert '"TestModel" object has no field "b"' in exc_info.value.args[0] | ||
|
||
else: | ||
assert m.a == 10 | ||
m.a = 11 | ||
assert m.a == 11 | ||
with pytest.raises(ValueError) as exc_info: | ||
m.b = 11 | ||
assert '"TestModel" object has no field "b"' in exc_info.value.args[0] | ||
|
||
|
||
def test_immutability(): | ||
def test_not_frozen_are_not_hashable(): | ||
class TestModel(BaseModel): | ||
a: int = 10 | ||
|
||
m = TestModel() | ||
with pytest.raises(TypeError) as exc_info: | ||
hash(m) | ||
assert "unhashable type: 'TestModel'" in exc_info.value.args[0] | ||
|
||
|
||
def test_frozen_with_hashable_fields_are_hashable(): | ||
class TestModel(BaseModel): | ||
a: int = 10 | ||
|
||
class Config: | ||
allow_mutation = False | ||
extra = Extra.forbid | ||
frozen = True | ||
|
||
m = TestModel() | ||
assert m.__hash__ is not None | ||
assert isinstance(hash(m), int) | ||
|
||
|
||
def test_frozen_with_unhashable_fields_are_not_hashable(): | ||
class TestModel(BaseModel): | ||
a: int = 10 | ||
y: List[int] = [1, 2, 3] | ||
|
||
class Config: | ||
frozen = True | ||
|
||
m = TestModel() | ||
assert m.a == 10 | ||
with pytest.raises(TypeError) as exc_info: | ||
m.a = 11 | ||
assert '"TestModel" is immutable and does not support item assignment' in exc_info.value.args[0] | ||
with pytest.raises(ValueError) as exc_info: | ||
m.b = 11 | ||
assert '"TestModel" object has no field "b"' in exc_info.value.args[0] | ||
hash(m) | ||
assert "unhashable type: 'list'" in exc_info.value.args[0] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO would be nice for the exception message to be more like "unhashable type: 'TestModel' contains unhashable type: 'list' but I wouldn't say it's a strong opinion. Food for thought. |
||
|
||
|
||
def test_hash_function_give_different_result_for_different_object(): | ||
class TestModel(BaseModel): | ||
a: int = 10 | ||
|
||
class Config: | ||
frozen = True | ||
|
||
m = TestModel() | ||
m2 = TestModel() | ||
m3 = TestModel(a=11) | ||
assert hash(m) == hash(m2) | ||
assert hash(m) != hash(m3) | ||
|
||
# Redefined `TestModel` | ||
class TestModel(BaseModel): | ||
a: int = 10 | ||
|
||
class Config: | ||
frozen = True | ||
|
||
m4 = TestModel() | ||
assert hash(m) != hash(m4) | ||
|
||
|
||
def test_const_validates(): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be removed too?