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

dataclass is not Hashable #11463

Open
KotlinIsland opened this issue Nov 5, 2021 · 5 comments · May be fixed by #13187
Open

dataclass is not Hashable #11463

KotlinIsland opened this issue Nov 5, 2021 · 5 comments · May be fixed by #13187
Labels
bug mypy got something wrong topic-dataclasses

Comments

@KotlinIsland
Copy link
Contributor

from dataclasses import dataclass
from typing import Hashable


@dataclass
class SusDataclass:
    b = "AMONGUS"


h: Hashable = SusDataclass()  # SUS ALERT!

d = {
    SusDataclass(): 1  # SUS ALERT! fails at runtime
}
@KotlinIsland KotlinIsland added the bug mypy got something wrong label Nov 5, 2021
@sobolevn
Copy link
Member

sobolevn commented Nov 6, 2021

This happens because unsafe_hash is not taken into an account in plugins/dataclasses.py:

decorator_arguments = {
'init': _get_decorator_bool_argument(self._ctx, 'init', True),
'eq': _get_decorator_bool_argument(self._ctx, 'eq', True),
'order': _get_decorator_bool_argument(self._ctx, 'order', False),
'frozen': _get_decorator_bool_argument(self._ctx, 'frozen', False),
}

This should still be Hashable:

from dataclasses import dataclass

@dataclass(unsafe_hash=True)
class Some:
    x: int

print(hash(Some(1)))  # ok

I will send a fix today. Thanks for the report! 👍

@sobolevn
Copy link
Member

sobolevn commented Nov 6, 2021

By the way, here's a note from the docs:

If hash() is not explicitly defined, or if it is set to None, then dataclass() may add an implicit hash() method. Although not recommended, you can force dataclass() to create a hash() method with unsafe_hash=True. This might be the case if your class is logically immutable but can nonetheless be mutated. This is a specialized use case and should be considered carefully.

https://docs.python.org/3/library/dataclasses.html#dataclasses.dataclass

So, it still might be hashable even without unsafe_hash. I will research it further.

@sobolevn
Copy link
Member

sobolevn commented Nov 6, 2021

unsafe_hash: If False (the default), a hash() method is generated according to how eq and frozen are set.

# __hash__

#    +------------------- 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 already have __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.

https://github.com/python/cpython/blob/e9594f6747eaaaa848c26e2bf67d467aabfd62b3/Lib/dataclasses.py#L121

@sobolevn
Copy link
Member

sobolevn commented Nov 6, 2021

I will send a PR after #11483 is merged. I don't want to solve merge conflicts.

@tmke8
Copy link
Contributor

tmke8 commented Nov 7, 2021

Aren't the docs quite clear about this?

By default, dataclass() will not implicitly add a __hash__() method unless it is safe to do so. [...] Here are the rules governing implicit creation of a __hash__() method. [...] If eq and frozen are both true, by default dataclass() will generate a __hash__() method for you. If eq is true and frozen is false, __hash__() will be set to None, marking it unhashable (which it is, since it is mutable). If eq is false, __hash__() will be left untouched meaning the __hash__() method of the superclass will be used (if the superclass is object, this means it will fall back to id-based hashing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-dataclasses
Projects
None yet
4 participants