[Python] Change the behaviour of __eq__

[AlenkaF]

Describe the enhancement requested

As arithmetics will be supported in #48085 we should also revisit __eq__ implementation and change the behaviour of the current approach for the use of a kernel function (pyarrow.compute.equal).

See:

• [Python] Support arithmetic on arrays and scalars #32007 (comment)
• GH-32007: [Python] Support arithmetic on arrays and scalars #48085 (comment)
• GH-32007: [Python] Support arithmetic on arrays and scalars #48085 (comment)

Component(s)

Python

[rmnskb]

take

[rmnskb]

@AlenkaF thanks for raising this issue, will work on this as discussed.

[rmnskb]

@AlenkaF I took a closer look at the issue, and right now the __eq__ dunder leverages custom implementation that many tests already rely on, e.g. two null arrays are equal etc. Should we deprecate this functionality and rely only on pc.equal kernel and its implementation or handle these cases inside the __eq__ body? Thanks!

[AlenkaF]

Yes, this is a bit tricky.
I think we should first only add a deprecation warning in the docstrings of the dunde, something like:

    """
    .. deprecated:: 23.0.0
       The behavior of `__eq__` will change in PyArrow 25.0.0 to use
       `pc.equal` kernel which handles nulls differently. To preserve
       current behaviour, use the `.equals()` method explicitly.
    
    See Also
    --------
    equals : Current equality behaviour (preserved method)
    """

and then open a separate issue (subtasks in this issue can also be used) that will actually change the behaviour of the method after a longer deprecation period. That is when our tests will also need to be changed.

cc @pitrou

[pitrou]

I don't know if we really want to change __eq__. @jorisvandenbossche What do you think?

[pitrou]

Also @ngoldbaum what is NumPy's experience with changing __eq__ semantics? Do you think that is a good idea overall, or would you recommend against it?

[jorisvandenbossche]

Personally I think it is a pity that we implemented __eq__ as an alias of the object equality .equals() method as I would prefer the element-wise operation, but given the longstanding behaviour ..
(EDIT: it's actually not such longstanding as I thought, because it seems I added this myself for Array in 2020: #7737)

Given that we are adding the other arithmetic dunder methods, I think I would lean towards also doing that for __eq__. But that will first require a deprecation for quite a while before we can change that.

[pitrou]

There's one complication that we should be careful about: if we change the meaning the __eq__, then the use of equality in boolean context will result in calling the __bool__ of the result array (which is always true unless the array is empty). if x == y would then always succeed if the arrays are not empty, which I hope we agree is extremely error-prone:

>>> bool(np.array([False, False, False]))
Traceback (most recent call last):
  Cell In[15], line 1
    bool(np.array([False, False, False]))
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

>>> bool(pa.array([False, False, False]))
True

So, if we change the meaning the __eq__ on arrays, we must also deprecate __bool__ like NumPy did. The cost and annoyance of that change may outweigh the benefits of different __eq__ semantics.

Perhaps it would be less disruptive if __eq__ continues doing the Pythonic thing of returning a bool, and equals is changed to call the compute function. But that would make us different from NumPy (is that a problem?).

[AlenkaF]

Oh, I see

In [14]: b = None

In [15]: a == b
Out[15]: False

In [16]: import pyarrow.compute as pc

In [17]: pc.equal(a,b)
Out[17]: 
<pyarrow.lib.BooleanArray object at 0x12eb27340>
[
  null,
  null,
  null
]

In [18]: bool(pc.equal(a,b))
Out[18]: True

In [19]: bool(a == b)
Out[19]: False

I think the first option of not falling back to the length of the array (the NumPy "way") is cleaner though I don't really see the huge cost and annoyance that might bring (only slight :) ). The other option, which is changing equals and keeping __eq__ as is, would also be OK with me, if we see the need to be less disruptive.

[jorisvandenbossche]

So, if we change the meaning the __eq__ on arrays, we must also deprecate __bool__ like NumPy did.

Good point, if we change __eq__, we should definitely also change __bool__.

That makes that the deprecation has an even wider impact, but personally I don't really mind the "raise error in bool(..)" behaviour like NumPy has as the end result.

[rmnskb]

@pitrou @jorisvandenbossche @AlenkaF
Thanks for your input!
I wanted to align on the expected course of action: should we just leave the things as is or do you want to change the behavior of both __eq__ and __bool__ to support the element-wise comparison? If latter is the case, I think we should also agree on the target release before thinking about deprecation.
I want to reach some kind of consensus before we commit to the implementation itself.

[ngoldbaum]

Sorry, I was on vacation when you pinged me and I missed this.

@seberg probably has more context on array truthiness and the community fallout. I came around to NumPy after they made the change.

That said, I think mostly the fallout was fixing bugs in user code.