Skip to content

bpo-32929: Dataclasses: Change the tri-state hash parameter to the boolean unsafe_hash. #5891

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.

Sign up for GitHub

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

Merged
ericvsmith merged 20 commits into from
Feb 26, 2018
Merged

bpo-32929: Dataclasses: Change the tri-state hash parameter to the boolean unsafe_hash. #5891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5998a25
Merge remote-tracking branch 'upstream/master'
ericvsmith Jan 24, 2018
900f699
Merge branch 'master' of github.com:ericvsmith/cpython
ericvsmith Jan 24, 2018
1609d08
Merge branch 'master' of github.com:python/cpython
ericvsmith Jan 25, 2018
8ae6c47
Merge branch 'master' of github.com:python/cpython
ericvsmith Jan 28, 2018
e0156c8
Merge branch 'master' of github.com:python/cpython
ericvsmith Jan 28, 2018
dd9be0d
Merge branch 'master' of github.com:python/cpython
ericvsmith Jan 28, 2018
ef7a3b5
Merge branch 'master' of github.com:python/cpython
ericvsmith Jan 28, 2018
356ca29
Merge branch 'master' of github.com:python/cpython
ericvsmith Feb 2, 2018
5f4a3cc
Merge branch 'master' of github.com:python/cpython
ericvsmith Feb 10, 2018
d8ef404
Merge remote-tracking branch 'refs/remotes/origin/master'
ericvsmith Feb 12, 2018
dda3dbf
Merge branch 'master' of github.com:ericvsmith/cpython
ericvsmith Feb 13, 2018
4916fff
Merge branch 'master' of github.com:python/cpython
ericvsmith Feb 23, 2018
a1842c1
Checkpoint: No error if __hash__ already exists but is None.
ericvsmith Feb 24, 2018
2fe3499
Fix the test for __hash__ being explicitly defined in the class. This…
ericvsmith Feb 25, 2018
142e203
Added blurb.
ericvsmith Feb 25, 2018
8b4646f
Removed duplicate code: just use the lookup table.
ericvsmith Feb 25, 2018
991d822
Fixed some tests that were being skipped.
ericvsmith Feb 25, 2018
db98ece
Fixed hash table to match the key. Removed add*, since it's no longer…
ericvsmith Feb 25, 2018
a98cd93
Improve a comment.
ericvsmith Feb 26, 2018
caf402a
Add a link to an if-statement version of the _hash_action table.
ericvsmith Feb 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184 changes: 105 additions & 79 deletions Lib/dataclasses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@

# Conditions for adding methods. The boxes indicate what action the
# dataclass decorator takes. For all of these tables, when I talk
# about init=, repr=, eq=, order=, hash=, or frozen=, I'm referring
# to the arguments to the @dataclass decorator. When checking if a
# dunder method already exists, I mean check for an entry in the
# class's __dict__. I never check to see if an attribute is defined
# in a base class.
# about init=, repr=, eq=, order=, unsafe_hash=, or frozen=, I'm
# referring to the arguments to the @dataclass decorator. When
# checking if a dunder method already exists, I mean check for an
# entry in the class's __dict__. I never check to see if an
# attribute is defined in a base class.

# Key:
# +=========+=========================================+
Expand All @@ -34,11 +34,6 @@
# +---------+-----------------------------------------+
# | add | Generated method is added. |
# +---------+-----------------------------------------+
# | add* | Generated method is added only if the |
# | | existing attribute is None and if the |
# | | user supplied a __eq__ method in the |
# | | class definition. |
# +---------+-----------------------------------------+
# | raise | TypeError is raised. |
# +---------+-----------------------------------------+
# | None | Attribute is set to None. |
Expand Down Expand Up @@ -115,43 +110,36 @@

# __hash__

# +------------------- hash= parameter
# | +----------- eq= parameter
# | | +--- frozen= parameter
# | | |
# v v v | | |
# | no | yes | <--- class has __hash__ in __dict__?
# +=========+=======+=======+========+========+
# | 1 None | False | False | | | No __eq__, use the base class __hash__
# +---------+-------+-------+--------+--------+
# | 2 None | False | True | | | No __eq__, use the base class __hash__
# +---------+-------+-------+--------+--------+
# | 3 None | True | False | None | | <-- the default, not hashable
# +---------+-------+-------+--------+--------+
# | 4 None | True | True | add | add* | Frozen, so hashable
# +---------+-------+-------+--------+--------+
# | 5 False | False | False | | |
# +---------+-------+-------+--------+--------+
# | 6 False | False | True | | |
# +---------+-------+-------+--------+--------+
# | 7 False | True | False | | |
# +---------+-------+-------+--------+--------+
# | 8 False | True | True | | |
# +---------+-------+-------+--------+--------+
# | 9 True | False | False | add | add* | Has no __eq__, but hashable
# +---------+-------+-------+--------+--------+
# |10 True | False | True | add | add* | Has no __eq__, but hashable
# +---------+-------+-------+--------+--------+
# |11 True | True | False | add | add* | Not frozen, but hashable
# +---------+-------+-------+--------+--------+
# |12 True | True | True | add | add* | Frozen, so hashable
# +=========+=======+=======+========+========+
# +------------------- unsafe_hash= parameter
# | +----------- eq= parameter
# | | +--- frozen= parameter
# | | |
# v v v | | |
# | no | yes | <--- class has explicitly defined __hash__
# +=======+=======+=======+========+========+
# | False | False | False | | | No __eq__, use the base class __hash__
# +-------+-------+-------+--------+--------+
# | False | False | True | | | No __eq__, use the base class __hash__
# +-------+-------+-------+--------+--------+
# | False | True | False | None | | <-- the default, not hashable
# +-------+-------+-------+--------+--------+
# | False | True | True | add | | Frozen, so hashable, allows override
# +-------+-------+-------+--------+--------+
# | True | False | False | add | raise | Has no __eq__, but hashable
# +-------+-------+-------+--------+--------+
# | True | False | True | add | raise | Has no __eq__, but hashable
# +-------+-------+-------+--------+--------+
# | True | True | False | add | raise | Not frozen, but hashable
# +-------+-------+-------+--------+--------+
# | True | True | True | add | raise | Frozen, so hashable
# +=======+=======+=======+========+========+
# For boxes that are blank, __hash__ is untouched and therefore
# inherited from the base class. If the base is object, then
# id-based hashing is used.
# Note that a class may have already __hash__=None if it specified an
# __eq__ method in the class body (not one that was created by
# @dataclass).
# See _hash_action (below) for a coded version of this table.


# Raised when an attempt is made to modify a frozen class.
Expand Down Expand Up @@ -557,7 +545,45 @@ def _set_new_attribute(cls, name, value):
return False


def _process_class(cls, repr, eq, order, hash, init, frozen):
# Decide if/how we're going to create a hash function. Key is
# (unsafe_hash, eq, frozen, does-hash-exist). Value is the action to
# take.
# Actions:
# '': Do nothing.
# 'none': Set __hash__ to None.
# 'add': Always add a generated __hash__function.
# 'exception': Raise an exception.
#
# +-------------------------------------- unsafe_hash?
# | +------------------------------- eq?
# | | +------------------------ frozen?
# | | | +---------------- has-explicit-hash?
# | | | |
# | | | | +------- action
# | | | | |
# v v v v v
_hash_action = {(False, False, False, False): (''),
(False, False, False, True ): (''),
(False, False, True, False): (''),
(False, False, True, True ): (''),
(False, True, False, False): ('none'),
(False, True, False, True ): (''),
(False, True, True, False): ('add'),
(False, True, True, True ): (''),
(True, False, False, False): ('add'),
(True, False, False, True ): ('exception'),
(True, False, True, False): ('add'),
(True, False, True, True ): ('exception'),
(True, True, False, False): ('add'),
(True, True, False, True ): ('exception'),
(True, True, True, False): ('add'),
(True, True, True, True ): ('exception'),
}
# See https://bugs.python.org/issue32929#msg312829 for an if-statement
# version of this table.


def _process_class(cls, repr, eq, order, unsafe_hash, init, frozen):
# Now that dicts retain insertion order, there's no reason to use
# an ordered dict. I am leveraging that ordering here, because
# derived class fields overwrite base class fields, but the order
Expand Down Expand Up @@ -605,8 +631,14 @@ def _process_class(cls, repr, eq, order, hash, init, frozen):
# be inherited down.
is_frozen = frozen or cls.__setattr__ is _frozen_setattr

# Was this class defined with an __eq__? Used in __hash__ logic.
auto_hash_test= '__eq__' in cls.__dict__ and getattr(cls.__dict__, '__hash__', MISSING) is None
# Was this class defined with an explicit __hash__? Note that if
# __eq__ is defined in this class, then python will automatically
# set __hash__ to None. This is a heuristic, as it's possible
# that such a __hash__ == None was not auto-generated, but it
# close enough.
class_hash = cls.__dict__.get('__hash__', MISSING)
has_explicit_hash = not (class_hash is MISSING or
(class_hash is None and '__eq__' in cls.__dict__))

# If we're generating ordering methods, we must be generating
# the eq methods.
Expand Down Expand Up @@ -661,48 +693,38 @@ def _process_class(cls, repr, eq, order, hash, init, frozen):
if _set_new_attribute(cls, name,
_cmp_fn(name, op, self_tuple, other_tuple)):
raise TypeError(f'Cannot overwrite attribute {name} '
f'in {cls.__name__}. Consider using '
f'in class {cls.__name__}. Consider using '
'functools.total_ordering')

if is_frozen:
for name, fn in [('__setattr__', _frozen_setattr),
('__delattr__', _frozen_delattr)]:
if _set_new_attribute(cls, name, fn):
raise TypeError(f'Cannot overwrite attribute {name} '
f'in {cls.__name__}')
f'in class {cls.__name__}')

# Decide if/how we're going to create a hash function.
# TODO: Move this table to module scope, so it's not recreated
# all the time.
generate_hash = {(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'),
}[None if hash is None else bool(hash), # Force bool() if not None.
bool(eq),
bool(frozen)]['__hash__' in cls.__dict__]
hash_action = _hash_action[bool(unsafe_hash),
bool(eq),
bool(frozen),
has_explicit_hash]

# No need to call _set_new_attribute here, since we already know if
# we're overwriting a __hash__ or not.
if generate_hash == '':
if hash_action == '':
# Do nothing.
pass
elif generate_hash == 'none':
elif hash_action == 'none':
cls.__hash__ = None
elif generate_hash in ('fn', 'fn-x'):
if generate_hash == 'fn' or auto_hash_test:
flds = [f for f in field_list
if (f.compare if f.hash is None else f.hash)]
cls.__hash__ = _hash_fn(flds)
elif hash_action == 'add':
flds = [f for f in field_list if (f.compare if f.hash is None else f.hash)]
cls.__hash__ = _hash_fn(flds)
elif hash_action == 'exception':
# Raise an exception.
raise TypeError(f'Cannot overwrite attribute __hash__ '
f'in class {cls.__name__}')
else:
assert False, f"can't get here: {generate_hash}"
assert False, f"can't get here: {hash_action}"

if not getattr(cls, '__doc__'):
# Create a class doc-string.
Expand All @@ -716,21 +738,21 @@ def _process_class(cls, repr, eq, order, hash, init, frozen):
# underscore. The presence of _cls is used to detect if this
# decorator is being called with parameters or not.
def dataclass(_cls=None, *, init=True, repr=True, eq=True, order=False,
hash=None, frozen=False):
unsafe_hash=None, frozen=False):
"""Returns the same class as was passed in, with dunder methods
added based on the fields defined in the class.

Examines PEP 526 __annotations__ to determine fields.

If init is true, an __init__() method is added to the class. If
repr is true, a __repr__() method is added. If order is true, rich
comparison dunder methods are added. If hash is true, a __hash__()
method function is added. If frozen is true, fields may not be
assigned to after instance creation.
comparison dunder methods are added. If unsafe_hash is true, a
__hash__() method function is added. If frozen is true, fields may
not be assigned to after instance creation.
"""

def wrap(cls):
return _process_class(cls, repr, eq, order, hash, init, frozen)
return _process_class(cls, repr, eq, order, unsafe_hash, init, frozen)

# See if we're being called as @dataclass or @dataclass().
if _cls is None:
Expand Down Expand Up @@ -793,6 +815,7 @@ class C:
raise TypeError("asdict() should be called on dataclass instances")
return _asdict_inner(obj, dict_factory)


def _asdict_inner(obj, dict_factory):
if _is_dataclass_instance(obj):
result = []
Expand Down Expand Up @@ -832,6 +855,7 @@ class C:
raise TypeError("astuple() should be called on dataclass instances")
return _astuple_inner(obj, tuple_factory)


def _astuple_inner(obj, tuple_factory):
if _is_dataclass_instance(obj):
result = []
Expand All @@ -849,7 +873,8 @@ def _astuple_inner(obj, tuple_factory):


def make_dataclass(cls_name, fields, *, bases=(), namespace=None, init=True,
repr=True, eq=True, order=False, hash=None, frozen=False):
repr=True, eq=True, order=False, unsafe_hash=None,
frozen=False):
"""Return a new dynamically created dataclass.

The dataclass name will be 'cls_name'. 'fields' is an iterable
Expand All @@ -869,7 +894,7 @@ class C(Base):

For the bases and namespace parameters, see the builtin type() function.

The parameters init, repr, eq, order, hash, and frozen are passed to
The parameters init, repr, eq, order, unsafe_hash, and frozen are passed to
dataclass().
"""

Expand All @@ -894,7 +919,8 @@ class C(Base):
namespace['__annotations__'] = anns
cls = type(cls_name, bases, namespace)
return dataclass(cls, init=init, repr=repr, eq=eq, order=order,
hash=hash, frozen=frozen)
unsafe_hash=unsafe_hash, frozen=frozen)


def replace(obj, **changes):
"""Return a new object replacing specified fields with new values.
Expand Down
Loading