RichCompare and RichCompareBool

[Aaron Brady]

Hi,

In the source for 3.0.1, PyObject_RichCompareBool seems to perform an
extra check on identity that PyObjecct_RichCompare does not perform.

Here's the excerpt from RichCompareBool (line 612):

/* Quick result when objects are the same.
Guarantees that identity implies equality. */
if (v == w) {
if (op == Py_EQ)
return 1;
else if (op == Py_NE)
return 0;
}

res = PyObject_RichCompare(v, w, op);

The code for PyObject_RichCompare does not contain this, it doesn't
seem. Is it a bug?

 

[Aaron Brady]

Without looking at the code it sounds like a bug: identity doesn't always
imply equality.

It may be that the code you've found only gets called in specific cases
where you can make that assumption, but otherwise it should be removed.

For reference, here's what the docs say (3.0.1):

'''
PyObject* PyObject_RichCompare(PyObject *o1, PyObject *o2, int opid)
Return value: New reference.
Compare the values of o1 and o2 using the operation specified by opid,
which must be one of Py_LT, Py_LE, Py_EQ, Py_NE, Py_GT, or Py_GE,
corresponding to <, <=, ==, !=, >, or >= respectively. This is the
equivalent of the Python expression o1 op o2, where op is the operator
corresponding to opid. Returns the value of the comparison on success,
or NULL on failure.

int PyObject_RichCompareBool(PyObject *o1, PyObject *o2, int opid)
Compare the values of o1 and o2 using the operation specified by opid,
which must be one of Py_LT, Py_LE, Py_EQ, Py_NE, Py_GT, or Py_GE,
corresponding to <, <=, ==, !=, >, or >= respectively. Returns -1 on
error, 0 if the result is false, 1 otherwise. This is the equivalent
of the Python expression o1 op o2, where op is the operator
corresponding to opid.
'''

As you can see, the only difference is the statement of the return
value.

As an aside, though, it's interesting that '__eq__' &c. can return any
object, in addition to True and False. I hadn't thought of that.
'RichCompareBool' just calls 'PyObject_IsTrue' on the result.

 

[Terry Reedy]

Hi,

In the source for 3.0.1, PyObject_RichCompareBool seems to perform an
extra check on identity that PyObjecct_RichCompare does not perform.

To me, the existence of two functions suggests that they are *intended*
to act differently.

 

[Terry Reedy]

Mmm, but then the documentation is misleading and incomplete, at least.
PyObject_RichCompareBool is not equivalent of "o1 op o2" as it claims,
and both functions differ in more than return value (the only documented
difference).

Could be. I believe the C API docs have had must less testing by naive
users than the Python docs, and even those are not quite perfect yet.

 

[Aaron Brady]

Could be.  I believe the C API docs have had must less testing by naive
users than the Python docs, and even those are not quite perfect yet.

RichCompareBool is used 62 times in the source, minus declaration.
It's partly (by eye) used to test identity with Py_EQ. Then there
should be a different function, more like "IdCompareBool".

Duncan stated:

Without looking at the code it sounds like a bug: identity doesn't always
imply equality.

In such cases, should "a in [a]" be True or False?

Abstract example:
.... def __eq__( self, oth ):
.... if self is oth:
.... return False
.... return object.__eq__( self, oth )
....

a= A()
a==a False
[a]==[a]

True

'list' does seem to break its contract. However without a concrete
use, I can't make a judgment for a vote.

Also, did not receive Gabriel's post.

 

[Gabriel Genellina]

Also, did not receive Gabriel's post.

That's because I replied a month ago - and probably you had no idea what I
was talking about by that time.

(Sorry, I inadvertedly set the clock one month back. You didn't miss
anything)

 

[Aaron Brady]

That's because I replied a month ago - and probably you had no idea what I  
was talking about by that time.

(Sorry, I inadvertedly set the clock one month back. You didn't miss  
anything)

BoolRichCompare( one month ago )?

 

[Aaron Brady]

Could be.  I believe the C API docs have had must less testing by naive
users than the Python docs, and even those are not quite perfect yet.

RichCompareBool is used 62 times in the source, minus declaration.
It's partly (by eye) used to test identity with Py_EQ.  Then there
should be a different function, more like "IdCompareBool".

Duncan stated:

Without looking at the code it sounds like a bug: identity doesn't always
imply equality.

In such cases, should "a in [a]" be True or False?

Abstract example:

...     def __eq__( self, oth ):
...         if self is oth:
...             return False
...         return object.__eq__( self, oth )
...>>> a= A()

a==a False
[a]==[a]

True

'list' does seem to break its contract.

....

Hi. Just bringing it up again. I feel the docs should mention it at
least, and there should possibly be a separate function.

 

[Steven D'Aprano]

Hi.  Just bringing it up again.  I feel the docs should mention it at
least, and there should possibly be a separate function.

Post a bug report or feature request on the tracker, or nothing will happen.

If you include a patch, odds of it being approved are greatly increased.

 

[Aaron Brady]

Post a bug report or feature request on the tracker, or nothing will happen.

If you include a patch, odds of it being approved are greatly increased.

I'm not quite sure how to handle it. The existing function is useful,
widely used, and misnamed. The existing calls should call the
auxiliary function, and RichCompareBool should get a new
implementation.

Actually, I question whether all of them are correctly calling the
auxiliary anyway.

 

[Aaron Brady]

Post a bug report or feature request on the tracker, or nothing will happen.

If you include a patch, odds of it being approved are greatly increased.

I submitted a patch some months ago for something else. It hasn't
been handled at all. Naturally, I am discouraged from submitting
further. What is your advice?

 

[Mark Dickinson]

The code for PyObject_RichCompare does not contain this, it doesn't
seem.  Is it a bug?

It's not a bug. See revision 67204:

http://svn.python.org/view?view=rev&revision=67204

Mark

 

[Aaron Brady]

It's not a bug.  See revision 67204:

http://svn.python.org/view?view=rev&revision=67204

Mark

My complaint was that the docs for the function, as well as its name,
are misleading. RichCompareBool should not take the short cut, and "x
in [x]" should call something else that does. (I am not arguing
against the decided behavior of "x in [x]", btw. There's no clear
case IMO.)

 

[Mark Dickinson]

My complaint was that the docs for the function, as well as its name,
are misleading.  RichCompareBool should not take the short cut, and "x
in [x]" should call something else that does.  (I am not arguing
against the decided behavior of "x in [x]", btw.  There's no clear
case IMO.)

I agree the docs are misleading. A doc patch would very likely
be accepted. Changing the behaviour or name of
PyObject_RichCompareBool would be trickier, since
that would risk breaking 3rd party extensions that depend
on the current behaviour.

Mark

 

[Aaron Brady]

My complaint was that the docs for the function, as well as its name,
are misleading.  RichCompareBool should not take the short cut, and "x
in [x]" should call something else that does.  (I am not arguing
against the decided behavior of "x in [x]", btw.  There's no clear
case IMO.)

I agree the docs are misleading.  A doc patch would very likely
be accepted.  Changing the behaviour or name of
PyObject_RichCompareBool would be trickier, since
that would risk breaking 3rd party extensions that depend
on the current behaviour.

Mark

Changing the docs for RichCompareBool is a different story from adding
a second function (and naming it). The docs should mention the
additional test.

A second function should be called something similar, but
RichCompareAsBool is probably too similar.

The actual name for RichCompareBool should be more like something like
ContainerMemberCompare. It's not that important that it needs be
phased out eventually, either, anyway.