Pyo3: Not raising a TypeError when it should after reversing the logical operator.

[mvaled]

When our implementation of a pyclass returns NotImplemented and the other side also returns NotImplemented, a TypeError must be raised.

This is maybe a consequence of the implementation for #1064.

The following code reproduces the issue:

    #[pyclass]
    #[derive(PartialEq, PartialOrd, Eq, Ord, Clone, Debug)]
    pub(crate) struct ExternalObject {
        name: String,
        id: u64,
    }

    #[pymethods]
    impl ExternalObject {
        #[new]
        fn new(name: String, id: u64) -> Self {
            Self { name, id }
        }
    }

    #[pyproto]
    impl<'p> PyObjectProtocol<'p> for ExternalObject {
        fn __richcmp__(&'p self, other: Self, op: CompareOp) -> PyResult<bool> {
            unimplemented!()
        }
    }

When calling from Python:

>>> ExternalObject("name", 1).__eq__(("name", 1))                                                                                                                                                                  
NotImplemented
>>> ("name", 1).__eq__(ExternalObject("name", 1))                                                                                                                                                                  
NotImplemented

>>> ExternalObject("name", 1) == ("name", 1)                                                                                                                                                                       
False

>>> ("name", 1) == ExternalObject("name", 1)                                                                                                                                                                       
False

Even though both calls to __eq__ return NotImplemented the operator == doesn't raise a TypeError and it should.

[kngwyu]

I'm sorry for the delay, but is it really true for ==?
I can't see TypeError in Python. Here's my attempt:

In [10]: class C:
    ...:     def __eq__(self, a):
    ...:         return NotImplemented

In [11]: 1 == C()
Out[11]: False

In [12]: C() == 1
Out[12]: False

But I'm sure this is true for some number methods and ~our current implementation is incorrect.~ (EDITED: I confirmed that it works)

In [10]: class C:
    ...:     def __eq__(self, a):
    ...:         return NotImplemented

In [11]: 1 == C()
Out[11]: False

In [12]: C() == 1
Out[12]: False

In [13]: class D:
    ...:     def __add__(self, a):
    ...:         return NotImplemented
    ...: 

In [14]: D() + D()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-14-715d6e17ac3a> in <module>
----> 1 D() + D()

TypeError: unsupported operand type(s) for +: 'D' and 'D'

[mvaled]

@kngwyu

I'm sorry for the delay, but is it really true for ==?

You're right. I was confused with the rest of operators.

However, for other comparison operators <, <=, etc, in Python we do have a TypeError. Our implementation works fine in such cases.

So, I'm closing this issue.

Sorry.

[davidhewitt]

👍 thanks for investigating this even if it turned out we're already correct. Easiest bugfix ever! 😄