From 095fbe9857962fb3d36cb3dbca1251cc4f2bf642 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Sat, 18 Feb 2017 07:16:52 +0100 Subject: [PATCH] Mirror cmp behavior --- CHANGELOG.rst | 3 ++- src/attr/_make.py | 24 +++++++++++------------- tests/test_dunders.py | 17 +++++++++++++++++ 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1513c07ca..feb73b835 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -31,7 +31,8 @@ Deprecations: Changes: ^^^^^^^^ -- Fix hashing behavior by setting ``__hash__`` to ``None`` by default and mirror ``cmp`` and ``hash`` in attributes. +- Fix default hashing behavior. + Now *hash* mirrors the value of *cmp* and classes are unhashable by default. `#136 `_ `#142 `_ - Add ``attr.evolve`` that, given an instance of an ``attrs`` class and field changes as keyword arguments, will instantiate a copy of the given instance with the changes applied. diff --git a/src/attr/_make.py b/src/attr/_make.py index b29020171..adca4cf2b 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -255,21 +255,19 @@ def attributes(maybe_cls=None, these=None, repr_ns=None, a tuple of its ``attrs`` attributes. But the attributes are *only* compared, if the type of both classes is *identical*! :param hash: If ``None`` (default), the ``__hash__`` method is generated - according how *cmp* and *frozen* are set. You will receive one if - *both* are ``True``. + according how *cmp* and *frozen* are set. + + 1. If *both* are True, ``attrs`` will generate a ``__hash__`` for you. + 2. If *cmp* is True and *frozen* is False, ``__hash__`` will be set to + None, marking it unhashable (which it is). + 3. If *cmp* is False, ``__hash__`` will be left untouched meaning the + ``__hash__`` method of the superclass will be used (if superclass is + ``object``, this means it will fall back to id-based hashing.). Although not recommended, you can decide for yourself and force ``attrs`` to create one (e.g. if the class is immutable even though you - didn't freeze it programmatically) by passing ``True`` or not (e.g. if - you want to use the superclass's ``__hash__`` method be it Python's - build-in id-based hashing or your own). Both of these cases are rather - special and should be used carefully. - - Please note that setting *hash* to ``False`` means that the - superclass's ``__hash__`` function is used. If you set it to ``None``, - and your class is not *both* ``cmp=True`` and ``frozen=True``, the - ``__hash__`` method is set to ``None``, making it not hashable (which - it is). + didn't freeze it programmatically) by passing ``True`` or not. Both of + these cases are rather special and should be used carefully. See the `Python documentation \ `_ @@ -340,7 +338,7 @@ def wrap(cls): raise TypeError( "Invalid value for hash. Must be True, False, or None." ) - elif hash is False: + elif hash is False or (hash is None and cmp is False): pass elif hash is True or (hash is None and cmp is True and frozen is True): cls = _add_hash(cls) diff --git a/tests/test_dunders.py b/tests/test_dunders.py index f27cceacf..9e24d3d11 100644 --- a/tests/test_dunders.py +++ b/tests/test_dunders.py @@ -272,6 +272,23 @@ def test_hash_attribute_mirrors_cmp(self, cmp): assert C(1) == C(2) assert hash(C(1)) == hash(C(2)) + @given(booleans()) + def test_hash_mirrors_cmp(self, cmp): + """ + If `hash` is None, the hash generation mirrors `cmp`. + """ + C = make_class("C", {"a": attr()}, cmp=cmp, frozen=True) + + i = C(1) + assert i == i + if cmp: + assert C(1) == C(1) + assert hash(C(1)) == hash(C(1)) + else: + assert C(1) != C(1) + assert hash(C(1)) != hash(C(1)) + assert hash(i) == hash(i) + @pytest.mark.parametrize("cls", [HashC, HashCSlots]) def test_hash_works(self, cls): """