Skip to content

Commit

Permalink
Generate a hash function when frozen is True (#1881)
Browse files Browse the repository at this point in the history
* feature: add a `frozen` parameter to config

For now, `frozen` is a strict duplication of `allow_mutation` parameter
i.e. setting `frozen=True` does everything that `allow_mutation=False` does.

NB: this does not change the behavior of `allow_mutation`.

In next commit, setting `frozen=True` will also make the BaseModel hashable
while the existing behavior of `allow_mutation` will not be updated.

* refactor: factorise immutability tests

* feature: generate a hash function when frozen is True

Now, setting `frozen=True` also generate a hash function for the model
i.e. `__hash__` is not `None`. This makes instances of the model potentially
hashable if all the attributes are hashable. (default: `False`)

* reviewer feedback: use hash of the class instead of the super

* reviewer feedback: fix spelling checks

* reviewer feedback: update changes description

* test: remwork mypy tests in order to catch only frozen related errors

Before: there were errors about other stuff than frozen behavior
After: The modification catch only errot related to the frozen behavior

* test: split test_immutablity in 2 functions

One function tests the behavior: 'the model is mutable'
The other tests the behavior:OC 'the model is immutable'

* test mutability: remove the unnecessary parametrization

* test immutability: remove assertion that do not test frozen behavior
  • Loading branch information
rhuille committed Feb 23, 2021
1 parent 7cc8d25 commit d8e8e6a
Show file tree
Hide file tree
Showing 10 changed files with 158 additions and 17 deletions.
2 changes: 2 additions & 0 deletions changes/1880-rhuille.md
@@ -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.
8 changes: 8 additions & 0 deletions docs/usage/model_config.md
Expand Up @@ -29,6 +29,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
13 changes: 10 additions & 3 deletions pydantic/main.py
Expand Up @@ -120,6 +120,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 @@ -215,12 +216,17 @@ def validate_custom_root_type(fields: Dict[str, ModelField]) -> None:
raise ValueError(f'{ROOT_KEY} cannot be mixed with other fields')


# Annotated fields can have many types like `str`, `int`, `List[str]`, `Callable`...
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


# If a field is of type `Callable`, its default value should be a function and cannot to ignored.
ANNOTATED_FIELD_UNTOUCHED_TYPES: Tuple[Any, ...] = (property, type, classmethod, staticmethod)
# When creating a `BaseModel` instance, we bypass all the methods, properties... added to the model
UNTOUCHED_TYPES: Tuple[Any, ...] = (FunctionType,) + ANNOTATED_FIELD_UNTOUCHED_TYPES

# Note `ModelMetaclass` refers to `BaseModel`, but is also used to *create* `BaseModel`, so we need to add this extra
# (somewhat hacky) boolean to keep track of whether we've created the `BaseModel` class yet, and therefore whether it's
# safe to refer to it. If it *hasn't* been created, we assume that the `__new__` call we're in the middle of is for
Expand Down Expand Up @@ -353,6 +359,7 @@ def is_untouched(v: Any) -> bool:
'__custom_root_type__': _custom_root_type,
'__private_attributes__': private_attributes,
'__slots__': slots | private_attributes.keys(),
'__hash__': generate_hash_function(config.frozen),
'__class_vars__': class_vars,
**{n: v for n, v in namespace.items() if n not in exclude_from_namespace},
}
Expand Down Expand Up @@ -413,7 +420,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
24 changes: 24 additions & 0 deletions tests/mypy/modules/plugin_fail.py
Expand Up @@ -202,3 +202,27 @@ class AddProject:


p = AddProject(name='x', slug='y', description='z')


# Same as Model, but with frozen = True
class FrozenModel(BaseModel):
x: int
y: str

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


frozenmodel = FrozenModel(x=1, y='b')
frozenmodel.y = 'a'


class InheritingModel2(FrozenModel):
class Config:
frozen = False


inheriting2 = InheritingModel2(x=1, y='c')
inheriting2.y = 'd'
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: int = 1

class Config:
frozen = False
orm_mode = True


NotFrozenModel(x=1).x = 2
NotFrozenModel.from_orm(model)
3 changes: 2 additions & 1 deletion tests/mypy/outputs/plugin-fail-strict.txt
Expand Up @@ -35,4 +35,5 @@
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>
219: error: Property "y" defined in "FrozenModel" is read-only [misc]
3 changes: 2 additions & 1 deletion tests/mypy/outputs/plugin-fail.txt
Expand Up @@ -24,4 +24,5 @@
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>
219: error: Property "y" defined in "FrozenModel" is read-only [misc]
19 changes: 18 additions & 1 deletion tests/test_construction.py
Expand Up @@ -253,7 +253,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 @@ -270,6 +270,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
77 changes: 68 additions & 9 deletions tests/test_main.py
Expand Up @@ -351,39 +351,98 @@ class Model(BaseModel):
assert exc_info.value.errors() == [{'loc': ('a',), 'msg': 'field required', 'type': 'value_error.missing'}]


def test_not_immutability():
def test_mutability():
class TestModel(BaseModel):
a: int = 10

class Config:
allow_mutation = True
extra = Extra.forbid
frozen = False

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]


def test_immutability():
@pytest.mark.parametrize('allow_mutation_, frozen_', [(False, False), (False, True), (True, True)])
def test_immutability(allow_mutation_, frozen_):
class TestModel(BaseModel):
a: int = 10

class Config:
allow_mutation = False
allow_mutation = allow_mutation_
extra = Extra.forbid
frozen = frozen_

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]


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:
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()
with pytest.raises(TypeError) as exc_info:
hash(m)
assert "unhashable type: 'list'" in exc_info.value.args[0]


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

0 comments on commit d8e8e6a

Please sign in to comment.