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

Generate a hash function when frozen is True #1881

Merged
1 change: 1 addition & 0 deletions changes/1880-rhuille.md
@@ -0,0 +1 @@
Add a new `frozen` boolean parameters. 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. (default: `False`)
rhuille marked this conversation as resolved.
Show resolved Hide resolved
8 changes: 8 additions & 0 deletions docs/usage/model_config.md
Expand Up @@ -26,6 +26,14 @@ Options:
**`allow_mutation`**
: whether or not models are faux-immutable, i.e. whether `__setattr__` is allowed (default: `True`)

**`frozen`**

!!! warning
This parameter is in beta

: 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. (default: `False`)


**`use_enum_values`**
: whether to populate models with the `value` property of enums, rather than the raw enum.
This may be useful if you want to serialise `model.dict()` later (default: `False`)
Expand Down
11 changes: 10 additions & 1 deletion pydantic/main.py
Expand Up @@ -112,6 +112,7 @@ class BaseConfig:
validate_all = False
extra = Extra.ignore
allow_mutation = True
frozen = False
allow_population_by_field_name = False
use_enum_values = False
fields: Dict[str, Union[str, Dict[str, str]]] = {}
Expand Down Expand Up @@ -198,6 +199,13 @@ def validate_custom_root_type(fields: Dict[str, ModelField]) -> None:
raise ValueError('__root__ cannot be mixed with other fields')


def generate_hash_function(frozen: bool) -> Optional[Callable[[Any], int]]:
def hash_function(self_: Any) -> int:
return hash(self_.__class__) + hash(tuple(self_.__dict__.values()))

return hash_function if frozen else None


UNTOUCHED_TYPES = FunctionType, property, type, classmethod, staticmethod

# Note `ModelMetaclass` refers to `BaseModel`, but is also used to *create* `BaseModel`, so we need to add this extra
Expand Down Expand Up @@ -321,6 +329,7 @@ def __new__(mcs, name, bases, namespace, **kwargs): # noqa C901
'__custom_root_type__': _custom_root_type,
'__private_attributes__': private_attributes,
'__slots__': slots | private_attributes.keys(),
'__hash__': generate_hash_function(config.frozen),
**{n: v for n, v in namespace.items() if n not in exclude_from_namespace},
}

Expand Down Expand Up @@ -374,7 +383,7 @@ def __setattr__(self, name, value): # noqa: C901 (ignore complexity)

if self.__config__.extra is not Extra.allow and name not in self.__fields__:
raise ValueError(f'"{self.__class__.__name__}" object has no field "{name}"')
elif not self.__config__.allow_mutation:
elif not self.__config__.allow_mutation or self.__config__.frozen:
raise TypeError(f'"{self.__class__.__name__}" is immutable and does not support item assignment')
elif self.__config__.validate_assignment:
new_values = {**self.__dict__, name: value}
Expand Down
7 changes: 5 additions & 2 deletions pydantic/mypy.py
Expand Up @@ -143,6 +143,7 @@ class PydanticModelTransformer:
tracked_config_fields: Set[str] = {
'extra',
'allow_mutation',
'frozen',
'orm_mode',
'allow_population_by_field_name',
'alias_generator',
Expand All @@ -159,7 +160,7 @@ def transform(self) -> None:
In particular:
* determines the model config and fields,
* adds a fields-aware signature for the initializer and construct methods
* freezes the class if allow_mutation = False
* freezes the class if allow_mutation = False or frozen = True
* stores the fields, config, and if the class is settings in the mypy metadata for access by subclasses
"""
ctx = self._ctx
Expand All @@ -174,7 +175,7 @@ def transform(self) -> None:
is_settings = any(get_fullname(base) == BASESETTINGS_FULLNAME for base in info.mro[:-1])
self.add_initializer(fields, config, is_settings)
self.add_construct_method(fields)
self.set_frozen(fields, frozen=config.allow_mutation is False)
self.set_frozen(fields, frozen=config.allow_mutation is False or config.frozen is True)
info.metadata[METADATA_KEY] = {
'fields': {field.name: field.serialize() for field in fields},
'config': config.set_values_dict(),
Expand Down Expand Up @@ -529,12 +530,14 @@ def __init__(
self,
forbid_extra: Optional[bool] = None,
allow_mutation: Optional[bool] = None,
frozen: Optional[bool] = None,
orm_mode: Optional[bool] = None,
allow_population_by_field_name: Optional[bool] = None,
has_alias_generator: Optional[bool] = None,
):
self.forbid_extra = forbid_extra
self.allow_mutation = allow_mutation
self.frozen = frozen
self.orm_mode = orm_mode
self.allow_population_by_field_name = allow_population_by_field_name
self.has_alias_generator = has_alias_generator
Expand Down
34 changes: 34 additions & 0 deletions tests/mypy/modules/plugin_fail.py
Expand Up @@ -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:
Copy link
Member

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?

pass

class Config:
alias_generator = None
frozen = True
extra = Extra.forbid

def config_method(self) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

do you need this?

...


frozenmodel = FrozenModel(x=1, y='y', z='z')
Copy link
Member

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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')
19 changes: 19 additions & 0 deletions tests/mypy/modules/plugin_success.py
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

add a type here to avoid extra errors in plugin-success-strict.txt


class Config:
frozen = False
orm_mode = True


NotFrozenModel(x=1).x = 2
NotFrozenModel.from_orm(model)
9 changes: 8 additions & 1 deletion tests/mypy/outputs/plugin-fail-strict.txt
Expand Up @@ -35,4 +35,11 @@
197: error: No overload variant of "dataclass" matches argument type "Dict[<nothing>, <nothing>]" [call-overload]
197: note: Possible overload variant:
197: note: def dataclass(*, init: bool = ..., repr: bool = ..., eq: bool = ..., order: bool = ..., unsafe_hash: bool = ..., frozen: bool = ..., config: Optional[Type[Any]] = ...) -> Callable[[Type[Any]], Type[Dataclass]]
197: note: <1 more non-matching overload not shown>
197: note: <1 more non-matching overload not shown>
224: error: Unexpected keyword argument "z" for "FrozenModel" [call-arg]
225: error: Missing named argument "y" for "FrozenModel" [call-arg]
226: error: Property "y" defined in "FrozenModel" is read-only [misc]
227: error: "FrozenModel" does not have orm_mode=True [pydantic-orm]
236: error: Argument "x" to "InheritingModel2" has incompatible type "str"; expected "int" [arg-type]
237: error: Argument "x" to "Settings" has incompatible type "str"; expected "int" [arg-type]
238: error: Argument "x" to "FrozenModel" has incompatible type "str"; expected "int" [arg-type]
6 changes: 5 additions & 1 deletion tests/mypy/outputs/plugin-fail.txt
Expand Up @@ -24,4 +24,8 @@
197: error: No overload variant of "dataclass" matches argument type "Dict[<nothing>, <nothing>]" [call-overload]
197: note: Possible overload variant:
197: note: def dataclass(*, init: bool = ..., repr: bool = ..., eq: bool = ..., order: bool = ..., unsafe_hash: bool = ..., frozen: bool = ..., config: Optional[Type[Any]] = ...) -> Callable[[Type[Any]], Type[Dataclass]]
197: note: <1 more non-matching overload not shown>
197: note: <1 more non-matching overload not shown>
224: error: Unexpected keyword argument "z" for "FrozenModel" [call-arg]
225: error: Missing named argument "y" for "FrozenModel" [call-arg]
226: error: Property "y" defined in "FrozenModel" is read-only [misc]
227: error: "FrozenModel" does not have orm_mode=True [pydantic-orm]
3 changes: 2 additions & 1 deletion tests/mypy/outputs/plugin-success-strict.txt
@@ -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]
19 changes: 18 additions & 1 deletion tests/test_construction.py
Expand Up @@ -223,7 +223,7 @@ def test_recursive_pickle():
assert m.__foo__ == m2.__foo__


def test_immutable_copy():
def test_immutable_copy_with_allow_mutation():
class Model(BaseModel):
a: int
b: int
Expand All @@ -240,6 +240,23 @@ class Config:
m2.b = 13


def test_immutable_copy_with_frozen():
class Model(BaseModel):
a: int
b: int

class Config:
frozen = True

m = Model(a=40, b=10)
assert m == m.copy()

m2 = m.copy(update={'b': 12})
assert repr(m2) == 'Model(a=40, b=12)'
with pytest.raises(TypeError):
m2.b = 13


def test_pickle_fields_set():
m = Model(a=24)
assert m.dict(exclude_unset=True) == {'a': 24}
Expand Down
91 changes: 74 additions & 17 deletions tests/test_main.py
Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The 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]
Copy link
Contributor

Choose a reason for hiding this comment

The 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():
Expand Down