opt_frozen_dataclass: Enable hashing with -O

[inducer]

Inspired by inducer/pytato#563.

cc @matthiasdiener (what do you think?)

[matthiasdiener]

Dunno, seems risky 😬

[matthiasdiener]

Maybe replacing unsafe_hash: bool = False, with unsafe_hash: bool = None, could be an option?

[inducer]

Dunno, seems risky 😬

I'd argue it's needed for correctness. Otherwise, these classes are hashable when running without -O and not hashable when running with -O, which is inconsistent. They're supposed to be semantically frozen classes, as the name suggests. I'll add a comment as to why that's there.

[inducer]

Maybe replacing unsafe_hash: bool = False, with unsafe_hash: bool = None, could be an option?

Sure, I could see that.

[matthiasdiener]

Dunno, seems risky 😬

I'd argue it's needed for correctness. Otherwise, these classes are hashable when running without -O and not hashable when running with -O, which is inconsistent. They're supposed to be semantically frozen classes, as the name suggests. I'll add a comment as to why that's there.

Part of the problem is that setting unsafe_hash=True disallows a custom hash function:

from dataclasses import dataclass

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

    def __hash__(self):
        return 42

$ python dc.py
Traceback (most recent call last):
  File "/Users/mdiener/Work/emirge/mirgecom/dc.py", line 4, in <module>
    @dataclass(unsafe_hash=True)
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mdiener/Work/emirge/miniforge3/envs/ceesd/lib/python3.11/dataclasses.py", line 1222, in wrap
    return _process_class(cls, init, repr, eq, order, unsafe_hash,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mdiener/Work/emirge/miniforge3/envs/ceesd/lib/python3.11/dataclasses.py", line 1091, in _process_class
    cls.__hash__ = hash_action(cls, field_list, globals)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mdiener/Work/emirge/miniforge3/envs/ceesd/lib/python3.11/dataclasses.py", line 851, in _hash_exception
    raise TypeError(f'Cannot overwrite attribute __hash__ '
TypeError: Cannot overwrite attribute __hash__ in class A

This error does not happen if just setting frozen=True, and I think this might be confusing to users of opt_frozen_dataclass.

[inducer]

Part of the problem is that setting unsafe_hash=True disallows a custom hash function:

Maybe that's playing whack-a-mole, but the latest revision addresses this.

[inducer]

Paging @alexfikl for a third set of eyes: Is all this just a terrible idea and not worth it?

[matthiasdiener]

Side note: this table is really helpful when thinking through this.

[matthiasdiener]

Suggested change

	this decorator avoid when the interpreter runs with "optimization"
	this decorator avoids when the interpreter runs with "optimization"

[matthiasdiener]

Suggested change

	The resulting dataclass supports hashing unless *eq* is set to *False*,
	if *unsafe_hash* is left at the default or set to *True*.
	The resulting dataclass supports hashing, even when it is not actually frozen,
	if *unsafe_hash* is left at the default or set to *True*.

[matthiasdiener]

Also, this is still hashable with eq=False, like normal dataclasses.

[matthiasdiener]

Besides the minor wording comments, this LGTM. Thanks!

[alexfikl]

Paging @alexfikl for a third set of eyes: Is all this just a terrible idea and not worth it?

Hm, I haven't followed this much, but if I understand correctly.. we want to turn off frozen when in -O mode because it's slower, but that also turns off hashing by default, right? And this is meant to sneakily add back hashing as if the class was frozen?

Assuming that's the intention, that sounds reasonable to me. The code looks fine now with @matthiasdiener's suggestions 😁

[inducer]

Thanks all for your help in getting this into shape. I'll do another release as soon as this is in.

[inducer]

https://pypi.org/project/pytools/2024.1.19/