bpo-32929: Dataclasses: Change the tri-state hash parameter to the boolean unsafe_hash. (GH-5891) (GH-5902)
unsafe_hash=False is now the default. It is the same behavior as the old hash=None parameter. unsafe_hash=True will try to add __hash__. If it already exists, TypeError is raised.
(cherry picked from commit dbf9cff48a4ad0fd58e1c623ce1f36c3dd3d5f38)
Co-authored-by: Eric V. Smith <ericvsmith@users.noreply.github.com>
diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py
index c24af7f..582cb34 100755
--- a/Lib/test/test_dataclasses.py
+++ b/Lib/test/test_dataclasses.py
@@ -83,32 +83,59 @@
class C(B):
x: int = 0
- def test_overwriting_hash(self):
+ def test_overwrite_hash(self):
+ # Test that declaring this class isn't an error. It should
+ # use the user-provided __hash__.
@dataclass(frozen=True)
class C:
x: int
def __hash__(self):
- pass
+ return 301
+ self.assertEqual(hash(C(100)), 301)
- @dataclass(frozen=True,hash=False)
- class C:
- x: int
- def __hash__(self):
- return 600
- self.assertEqual(hash(C(0)), 600)
-
+ # Test that declaring this class isn't an error. It should
+ # use the generated __hash__.
@dataclass(frozen=True)
class C:
x: int
- def __hash__(self):
- pass
+ def __eq__(self, other):
+ return False
+ self.assertEqual(hash(C(100)), hash((100,)))
- @dataclass(frozen=True, hash=False)
+ # But this one should generate an exception, because with
+ # unsafe_hash=True, it's an error to have a __hash__ defined.
+ with self.assertRaisesRegex(TypeError,
+ 'Cannot overwrite attribute __hash__'):
+ @dataclass(unsafe_hash=True)
+ class C:
+ def __hash__(self):
+ pass
+
+ # Creating this class should not generate an exception,
+ # because even though __hash__ exists before @dataclass is
+ # called, (due to __eq__ being defined), since it's None
+ # that's okay.
+ @dataclass(unsafe_hash=True)
class C:
x: int
- def __hash__(self):
- return 600
- self.assertEqual(hash(C(0)), 600)
+ def __eq__(self):
+ pass
+ # The generated hash function works as we'd expect.
+ self.assertEqual(hash(C(10)), hash((10,)))
+
+ # Creating this class should generate an exception, because
+ # __hash__ exists and is not None, which it would be if it had
+ # been auto-generated do due __eq__ being defined.
+ with self.assertRaisesRegex(TypeError,
+ 'Cannot overwrite attribute __hash__'):
+ @dataclass(unsafe_hash=True)
+ class C:
+ x: int
+ def __eq__(self):
+ pass
+ def __hash__(self):
+ pass
+
def test_overwrite_fields_in_derived_class(self):
# Note that x from C1 replaces x in Base, but the order remains
@@ -294,19 +321,6 @@
"not supported between instances of 'B' and 'C'"):
fn(B(0), C(0))
- def test_0_field_hash(self):
- @dataclass(hash=True)
- class C:
- pass
- self.assertEqual(hash(C()), hash(()))
-
- def test_1_field_hash(self):
- @dataclass(hash=True)
- class C:
- x: int
- self.assertEqual(hash(C(4)), hash((4,)))
- self.assertEqual(hash(C(42)), hash((42,)))
-
def test_eq_order(self):
# Test combining eq and order.
for (eq, order, result ) in [
@@ -407,25 +421,25 @@
# Test all 6 cases of:
# hash=True/False/None
# compare=True/False
- for (hash_val, compare, result ) in [
+ for (hash_, compare, result ) in [
(True, False, 'field' ),
(True, True, 'field' ),
(False, False, 'absent'),
(False, True, 'absent'),
(None, False, 'absent'),
(None, True, 'field' ),
- ]:
- with self.subTest(hash_val=hash_val, compare=compare):
- @dataclass(hash=True)
+ ]:
+ with self.subTest(hash=hash_, compare=compare):
+ @dataclass(unsafe_hash=True)
class C:
- x: int = field(compare=compare, hash=hash_val, default=5)
+ x: int = field(compare=compare, hash=hash_, default=5)
if result == 'field':
# __hash__ contains the field.
- self.assertEqual(C(5).__hash__(), hash((5,)))
+ self.assertEqual(hash(C(5)), hash((5,)))
elif result == 'absent':
# The field is not present in the hash.
- self.assertEqual(C(5).__hash__(), hash(()))
+ self.assertEqual(hash(C(5)), hash(()))
else:
assert False, f'unknown result {result!r}'
@@ -737,7 +751,7 @@
validate_class(C)
# Now repeat with __hash__.
- @dataclass(frozen=True, hash=True)
+ @dataclass(frozen=True, unsafe_hash=True)
class C:
i: int
j: str
@@ -1107,7 +1121,7 @@
self.assertEqual(C().x, [])
# hash
- @dataclass(hash=True)
+ @dataclass(unsafe_hash=True)
class C:
x: list = field(default_factory=list, hash=False)
self.assertEqual(astuple(C()), ([],))
@@ -2242,28 +2256,13 @@
pass
class TestHash(unittest.TestCase):
- def test_hash(self):
- @dataclass(hash=True)
+ def test_unsafe_hash(self):
+ @dataclass(unsafe_hash=True)
class C:
x: int
y: str
self.assertEqual(hash(C(1, 'foo')), hash((1, 'foo')))
- def test_hash_false(self):
- @dataclass(hash=False)
- class C:
- x: int
- y: str
- self.assertNotEqual(hash(C(1, 'foo')), hash((1, 'foo')))
-
- def test_hash_none(self):
- @dataclass(hash=None)
- class C:
- x: int
- with self.assertRaisesRegex(TypeError,
- "unhashable type: 'C'"):
- hash(C(1))
-
def test_hash_rules(self):
def non_bool(value):
# Map to something else that's True, but not a bool.
@@ -2273,89 +2272,73 @@
return (3,)
return 0
- def test(case, hash, eq, frozen, with_hash, result):
- with self.subTest(case=case, hash=hash, eq=eq, frozen=frozen):
- if with_hash:
- @dataclass(hash=hash, eq=eq, frozen=frozen)
- class C:
- def __hash__(self):
- return 0
- else:
- @dataclass(hash=hash, eq=eq, frozen=frozen)
- class C:
- pass
+ def test(case, unsafe_hash, eq, frozen, with_hash, result):
+ with self.subTest(case=case, unsafe_hash=unsafe_hash, eq=eq,
+ frozen=frozen):
+ if result != 'exception':
+ if with_hash:
+ @dataclass(unsafe_hash=unsafe_hash, eq=eq, frozen=frozen)
+ class C:
+ def __hash__(self):
+ return 0
+ else:
+ @dataclass(unsafe_hash=unsafe_hash, eq=eq, frozen=frozen)
+ class C:
+ pass
# See if the result matches what's expected.
- if result in ('fn', 'fn-x'):
+ if result == 'fn':
# __hash__ contains the function we generated.
self.assertIn('__hash__', C.__dict__)
self.assertIsNotNone(C.__dict__['__hash__'])
- if result == 'fn-x':
- # This is the "auto-hash test" case. We
- # should overwrite __hash__ iff there's an
- # __eq__ and if __hash__=None.
-
- # There are two ways of getting __hash__=None:
- # explicitely, and by defining __eq__. If
- # __eq__ is defined, python will add __hash__
- # when the class is created.
- @dataclass(hash=hash, eq=eq, frozen=frozen)
- class C:
- def __eq__(self, other): pass
- __hash__ = None
-
- # Hash should be overwritten (non-None).
- self.assertIsNotNone(C.__dict__['__hash__'])
-
- # Same test as above, but we don't provide
- # __hash__, it will implicitely set to None.
- @dataclass(hash=hash, eq=eq, frozen=frozen)
- class C:
- def __eq__(self, other): pass
-
- # Hash should be overwritten (non-None).
- self.assertIsNotNone(C.__dict__['__hash__'])
-
elif result == '':
# __hash__ is not present in our class.
if not with_hash:
self.assertNotIn('__hash__', C.__dict__)
+
elif result == 'none':
# __hash__ is set to None.
self.assertIn('__hash__', C.__dict__)
self.assertIsNone(C.__dict__['__hash__'])
+
+ elif result == 'exception':
+ # Creating the class should cause an exception.
+ # This only happens with with_hash==True.
+ assert(with_hash)
+ with self.assertRaisesRegex(TypeError, 'Cannot overwrite attribute __hash__'):
+ @dataclass(unsafe_hash=unsafe_hash, eq=eq, frozen=frozen)
+ class C:
+ def __hash__(self):
+ return 0
+
else:
assert False, f'unknown result {result!r}'
- # There are 12 cases of:
- # hash=True/False/None
+ # There are 8 cases of:
+ # unsafe_hash=True/False
# eq=True/False
# frozen=True/False
# And for each of these, a different result if
# __hash__ is defined or not.
- for case, (hash, eq, frozen, result_no, result_yes) in enumerate([
- (None, False, False, '', ''),
- (None, False, True, '', ''),
- (None, True, False, 'none', ''),
- (None, True, True, 'fn', 'fn-x'),
- (False, False, False, '', ''),
- (False, False, True, '', ''),
- (False, True, False, '', ''),
- (False, True, True, '', ''),
- (True, False, False, 'fn', 'fn-x'),
- (True, False, True, 'fn', 'fn-x'),
- (True, True, False, 'fn', 'fn-x'),
- (True, True, True, 'fn', 'fn-x'),
- ], 1):
- test(case, hash, eq, frozen, False, result_no)
- test(case, hash, eq, frozen, True, result_yes)
+ for case, (unsafe_hash, eq, frozen, res_no_defined_hash, res_defined_hash) in enumerate([
+ (False, False, False, '', ''),
+ (False, False, True, '', ''),
+ (False, True, False, 'none', ''),
+ (False, True, True, 'fn', ''),
+ (True, False, False, 'fn', 'exception'),
+ (True, False, True, 'fn', 'exception'),
+ (True, True, False, 'fn', 'exception'),
+ (True, True, True, 'fn', 'exception'),
+ ], 1):
+ test(case, unsafe_hash, eq, frozen, False, res_no_defined_hash)
+ test(case, unsafe_hash, eq, frozen, True, res_defined_hash)
# Test non-bool truth values, too. This is just to
# make sure the data-driven table in the decorator
# handles non-bool values.
- test(case, non_bool(hash), non_bool(eq), non_bool(frozen), False, result_no)
- test(case, non_bool(hash), non_bool(eq), non_bool(frozen), True, result_yes)
+ test(case, non_bool(unsafe_hash), non_bool(eq), non_bool(frozen), False, res_no_defined_hash)
+ test(case, non_bool(unsafe_hash), non_bool(eq), non_bool(frozen), True, res_defined_hash)
def test_eq_only(self):
@@ -2373,8 +2356,8 @@
self.assertNotEqual(C(1), C(4))
# And make sure things work in this case if we specify
- # hash=True.
- @dataclass(hash=True)
+ # unsafe_hash=True.
+ @dataclass(unsafe_hash=True)
class C:
i: int
def __eq__(self, other):
@@ -2384,7 +2367,7 @@
# And check that the classes __eq__ is being used, despite
# specifying eq=True.
- @dataclass(hash=True, eq=True)
+ @dataclass(unsafe_hash=True, eq=True)
class C:
i: int
def __eq__(self, other):
@@ -2393,10 +2376,35 @@
self.assertNotEqual(C(1), C(1))
self.assertEqual(hash(C(1)), hash(C(1.0)))
+ def test_0_field_hash(self):
+ @dataclass(frozen=True)
+ class C:
+ pass
+ self.assertEqual(hash(C()), hash(()))
+
+ @dataclass(unsafe_hash=True)
+ class C:
+ pass
+ self.assertEqual(hash(C()), hash(()))
+
+ def test_1_field_hash(self):
+ @dataclass(frozen=True)
+ class C:
+ x: int
+ self.assertEqual(hash(C(4)), hash((4,)))
+ self.assertEqual(hash(C(42)), hash((42,)))
+
+ @dataclass(unsafe_hash=True)
+ class C:
+ x: int
+ self.assertEqual(hash(C(4)), hash((4,)))
+ self.assertEqual(hash(C(42)), hash((42,)))
+
def test_hash_no_args(self):
# Test dataclasses with no hash= argument. This exists to
- # make sure that when hash is changed, the default hashability
- # keeps working.
+ # make sure that if the @dataclass parameter name is changed
+ # or the non-default hashing behavior changes, the default
+ # hashability keeps working the same way.
class Base:
def __hash__(self):