Incompatibility between _decimal and _pydecimal: Context.__hash__

[srittau]

Bug report

_decimal.Context.__hash__() returns an int value, while _pydecimal.Context.__hash__() is explicitly disabled:

Python 3.10.5 (main, Jun  7 2022, 12:59:15) [GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from _decimal import Context
>>> Context().__hash__()
8743628939711
>>> from _pydecimal import Context
>>> Context().__hash__()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'NoneType' object is not callable

I assume that _decimal just didn't bother to override __hash__ and inherits its implementation from object, but I think the implementations should match.

Your environment

Tested:

• Python 3.10.5 (main, Jun 7 2022, 12:59:15) [GCC 10.2.1 20210110] on linux
• Python 3.9.2 (default, Feb 28 2021, 17:03:44)

Linked PRs

• gh-94107: Make _pydecimal.Context hashable to match _decimal.Context #158160

[kumaraditya303]

cc @mdickinson
I think that in the C implementation PyObject_HashNotImplemented should be added as tp_hash slot.

[mdickinson]

I think we risk running into Hyrum's law here: making _decimal.Context objects unhashable would be a backwards-incompatible change, and there's almost certainly someone depending on them being hashable (even if only accidentally) somewhere. If we really want to do that then we should probably go through a deprecation period, but it doesn't seem worth the trouble - I don't think the hashability is causing any real issues.

[mdickinson]

FWIW, the non-hashability of contexts in the original Python version of decimal.py was intentional: see

cpython/Lib/decimal.py

Lines 2200 to 2203 in 5aa478b

	def __hash__(self):
	"""A Context cannot be hashed."""
	# We inherit object.__hash__, so we must deny this explicitly
	raise TypeError, "Cannot hash a Context."

[srittau]

I think we risk running into Hyrum's law here: making _decimal.Context objects unhashable would be a backwards-incompatible change, and there's almost certainly someone depending on them being hashable (even if only accidentally) somewhere. If we really want to do that then we should probably go through a deprecation period, but it doesn't seem worth the trouble - I don't think the hashability is causing any real issues.

But please note that if someone is depending on the hashability already, the code is already subtly broken, as it won't work on systems, where _decimal is not available. In that case, _pydecimal.Context.__hash__ should be removed.

[mdickinson]

In that case, _pydecimal.Context.__hash__ should be removed.

Mmm, possibly. That feels like a step in the wrong direction to me.

I do agree in principle that Context objects should be unhashable. It just doesn't seem worth risking breakage of existing code in pursuit of that principle, when the proposed change wouldn't provide any benefits (at least, not that I can see) to offset the possibility of breakage. Practicality beats purity, and all that.

Anyway, I'll defer to @rhettinger on this one: it was his decision originally to make Context objects unhashable.

[rhettinger]

The reason the original Python version turned-off hashing is that that was the norm for mutable objects, following the example of list, set, and dict. Also at the time, we kept open the possibility of adding a custom __eq__ method to Context objects to make it easier to detect changes in state.

PEP 399 says that we should treat the difference between the C version and Python version as a bug and fix it.

Changing the C code to match the Python version is the right thing to do in principle, but I agree with Mark that some code may be relying on C version being hashable. The least disruptive thing we can do it is remove the __hash__ method in the Python code so that it too inherits object.__hash__. But if you all want to go the other way and alter the C code to match the Python code, that would fine as well.

[mdickinson]

I'd be happy to review a PR that goes either direction: removing Context.__hash__ in _pydecimal as the easiest way to satisfy PEP 399, or deprecating Context.__hash__ in _decimal as the "right" way, that makes cdecimal match the original intent.