_libs.hashtable.ismember incorrect with tuples containing nan

[jbrockmendel]

It looks like the equality check being done on tuples is checking NA values for identity, so separately instantiated float("nan") objects aren't considered matching.

from pandas.core.algorithms import isin

values = [("a", float("nan")), ("b", 1)]
comps = [("a", float("nan"))]
alt = ("a", values[0][1])

>>> isin(values, comps)
array([False, False])

>>> isin(values, [alt])
array([ True, False])

cc @realead

[realead]

That is true. The reason is that

float("nan") == float("nan")

is False in Python.

We overwrite default __eq__ for floats:

pandas/pandas/_libs/src/klib/khash_python.h

Lines 172 to 177 in 059c8ba

	if (result == 0) { // still could be two NaNs
	return PyFloat_CheckExact(a) &&
	PyFloat_CheckExact(b) &&
	Py_IS_NAN(PyFloat_AS_DOUBLE(a)) &&
	Py_IS_NAN(PyFloat_AS_DOUBLE(b));
	}

, but not other types.

To be consistent, we should do the same for other built-in types (complex, tuple, frozenset.... are there more?). I would not expect too much impact on performace.

I don't see an easy way to ensure the correct behavior for non-built-in classes like

class A:
    def __init__(self, a):
        self.a=a
    def __eq__(self, other):
        return self.a == other.a

and would say it is responsibility of the class' authors to ensure the desired behavior.

None and pd.NaT seems to have no issue, because they are singletones and Python short-cuts comparison of an object with itself.

[realead]

@jbrockmendel I'have looked into this problem. The fix is relatively simple for complex and tuple (here is an prototype: https://github.com/realead/pandas/tree/fix_deep_nans).

However, the situation is quite different for frozenset, we need to replace the following comparison in the implementation of the setobject:

static setentry *
set_lookkey(PySetObject *so, PyObject *key, Py_hash_t hash)
{
       ...
       cmp = PyObject_RichCompareBool(startkey, key, Py_EQ);
       ....
}

but there are so many implemenation details in this function, that it would be really unwise to duplicate the code.

A sound fix would be to introduce a Py_EQ_FOR_HASH_TABLE to CPython's richcompare (which would be different from PY_EQ for floats as it would say that float("nan") is equal float("nan")). But this obviuosly is out of reach.

So my proposal would be to add special handling of complex and tuple objects and to leave the situation with frozenset as it is.

[jbrockmendel]

Thanks for looking into this. The only case that is a showstopper for me ATM is the tuple, so i think punting on frozenset is totally reasonable.

[realead]

Because of python/cpython@a07da09 we need to change the hash-function as well, otherwise different nans will have different hashes for Py3.10 and later.