etils icon indicating copy to clipboard operation
etils copied to clipboard

edc: unfrozen dataclasses are hashble

Open SobhanMP opened this issue 2 years ago • 2 comments

I think the general consensus in Python is that mutable types should not be hashable; however unfrozen dataclasses are hashable. See code below:

import dataclasses
from etils import edc
@edc.dataclass(allow_unfrozen=True)
@dataclasses.dataclass(eq=True, kw_only=True, frozen=True)
class Conf:
    x: int = 3
a = Conf()
a = a.unfrozen()
a.x = 2
hash(a) # should fail

ps. edc is really handy

SobhanMP avatar Jul 19 '23 18:07 SobhanMP

Sorry for the late answer, I somehow missed this issue.

Thank you for the feedback. I'm not sure about this one. Most python objects are hashable by default, including mutable ones.

a = object()
a.x = 2  # a is mutable
hash(a)  # Works

Is this an issue in practice ? Was there a failure you encounter because of this ? Otherwise I'll be in favour of keeping the default. It is sometimes convenient to use the default identity based hash, though I understand in this specific case, this could be confusing.

Conchylicultor avatar Oct 10 '23 15:10 Conchylicultor

It's not really an issue, but I imagine it makes for an easy-to-make error for jitted jax functions.

SobhanMP avatar Oct 10 '23 19:10 SobhanMP