RichCompare and RichCompareBool

Discussion in 'Python' started by Aaron Brady, Mar 2, 2009.

  1. Aaron Brady

    Aaron Brady Guest

    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, Mar 2, 2009
    #1
    1. Advertisements

  2. Aaron Brady

    Aaron Brady Guest

    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.
     
    Aaron Brady, Mar 2, 2009
    #2
    1. Advertisements

  3. Aaron Brady

    Terry Reedy Guest

    To me, the existence of two functions suggests that they are *intended*
    to act differently.
     
    Terry Reedy, Mar 2, 2009
    #3
  4. Aaron Brady

    Terry Reedy Guest

    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.
     
    Terry Reedy, Mar 3, 2009
    #4
  5. Aaron Brady

    Aaron Brady Guest

    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:
    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 )
    ....
    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.
     
    Aaron Brady, Mar 3, 2009
    #5
  6. 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)
     
    Gabriel Genellina, Mar 3, 2009
    #6
  7. Aaron Brady

    Aaron Brady Guest

    BoolRichCompare( one month ago )?
     
    Aaron Brady, Mar 3, 2009
    #7
  8. Aaron Brady

    Aaron Brady Guest

    ....

    Hi. Just bringing it up again. I feel the docs should mention it at
    least, and there should possibly be a separate function.
     
    Aaron Brady, Mar 7, 2009
    #8
  9. 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.
     
    Steven D'Aprano, Mar 8, 2009
    #9
  10. Aaron Brady

    Aaron Brady Guest

    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, Mar 8, 2009
    #10
  11. Aaron Brady

    Aaron Brady Guest

    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?
     
    Aaron Brady, Mar 9, 2009
    #11
  12. Mark Dickinson, Mar 9, 2009
    #12
  13. Aaron Brady

    Aaron Brady Guest

    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.)
     
    Aaron Brady, Mar 9, 2009
    #13
  14. 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
     
    Mark Dickinson, Mar 9, 2009
    #14
  15. Aaron Brady

    Aaron Brady Guest

    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.
     
    Aaron Brady, Mar 9, 2009
    #15
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.
Similar Threads
There are no similar threads yet.
Loading...