Opened 8 years ago

Last modified 22 months ago

#16537 closed enhancement

Python 3 preparation: Use "rich comparison" - std function cmp() is gone, method __cmp__ is ignored — at Version 14

Reported by: wluebbe Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: python3 Keywords: python3, richcmp
Cc: jpflori Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #17175 Stopgaps:

Status badges

Description (last modified by chapoton)

Since Py2.1 there are the rich comparison special methods (__eq__, __ne__, __lt__, __le__, __gt__, __ge__) to implement comparison operators (==, !=, <, <=, >, >= respectively) for custom classes.

This is more flexible (but somewhat more tedious) than defining the special method __cmp__.

In Py3 the special method __cmp__ is ignored. The standard function cmp() (which is mostly used to implement __cmp__ methods) is gone.

One may define a replacement function like

def cmp(a, b):
    return (a > b) - (a < b)

But this does not improved performance. And it does not seem forward looking ...

Unfortunately __cmp__ and cmp() are used a LOT in Sage. And the migration to rich comparison can not be done purely mechanical :-(

Since Py2.7 there is a class decorator functools.total_ordering to help to create the full set of special method __lt__, __le__, __gt__, __ge__ when given one of those.
See Lennart Regebro's Chapter: Use rich comparison operators for a more detailed discussion.

Useful tool to see the size of the remaining problem:

git grep -c "^[^#]*[^a-z\._]cmp(" *.py
git grep -c "^[^#]*[^a-z\._]cmp(" *.pyx

This ticket is tracked as a dependency of meta-ticket ticket:15980.

Change History (23)

comment:1 Changed 8 years ago by wluebbe

I did some grepping to get a first impression of the amount of work required:

git grep -P "def\s+__cmp__" |wc -l 377
git grep -P "def\s+__eq__" |wc -l 170
git grep -P "def\s+__ne__" |wc -l 70
git grep -P "def\s+__lt__" |wc -l 29
git grep -P "def\s+__le__" |wc -l 18
git grep -P "def\s+__gt__" |wc -l 21
git grep -P "def\s+__ge__" |wc -l 18
git grep -P "\bcmp\s*\(" |wc -l 1042

The conclusion seems that we have to implement rich comparison (i.e. define __eq__ with __ne__ and __gt__ with the class decorator functools.total_ordering) for about 350 classes!

And then there are those 1000+ calls to cmp.

And unfortunately the changes are mostly not syntactical but they require insight into the code!

Changed 8 years ago by wluebbe

Python script to demonstrate some "features" of rich comparison.

Changed 8 years ago by wluebbe

Changed 8 years ago by wluebbe

Changed 8 years ago by wluebbe

Changed 8 years ago by wluebbe

Changed 8 years ago by wluebbe

Changed 8 years ago by wluebbe

Changed 8 years ago by wluebbe

Changed 8 years ago by wluebbe

comment:2 Changed 8 years ago by wluebbe

Some remarks on the attached files:

  • The 1st script shows the effects of varying which of the special methods __eq__, __ne__, __lt__ and @total_ordering are defined. The 2nd and 3rd files show the results for Py2 and Py3 respectively.
    • In Py2 all of ("eq", "ne", "lt", "total") must be used to always obtain correct results.
    • In Py3 it appears that "ne" is implied (be "eq").
    • For correct behavior in Py2 and Py3 all of "eq", "ne", "lt", "total" should be used.
  • The 4th script shows the results of the comparison operators when defining no __cmp_ or using a self defined cmp or using the standard library cmp. The 5th and 6th files show the results for Py2 and Py3 respectively.
    • In Py2 "no_cmp" gives often wrong results, but always correct results when __cmp__ is defined.
    • In Py3 there are always type errors for "lt", "le", "gt"and "ge". For "eq" and "ne" some wrong and some correct.
    • __cmp__ does not work in Py3! Rich comparison must be implemented!
  • The 7th script shows whether it possible to define the rich comparison special methods using cmp. The 8nd and 9th files show the results for Py2 and Py3 respectively.
    • In Py2 it is possible.
    • In Py3 one needs a user defined cmp function that is based on < and >.
    • Both in Py2 and Py3 @total_ordering is required.

comment:3 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:4 Changed 8 years ago by aapitzsch

  • Dependencies set to #17175

comment:5 follow-ups: Changed 7 years ago by rws

Can the work be reduced by including the code in http://www.voidspace.org.uk/python/articles/comparison.shtml into the SageObject class?

comment:6 in reply to: ↑ 5 Changed 7 years ago by nbruin

Replying to rws:

Can the work be reduced by including the code in http://www.voidspace.org.uk/python/articles/comparison.shtml into the SageObject class?

I suspect that sage.structure.element.Element is a better location for it, since SageObject doesn't have any comparison infrastructure, and Element has.

In any case, the quoted code cannot be used verbatim, since these classes are cdef cython, where the interface is via the __richcmp__ special method, not the various __eq__ etc.

Element already has a __richcmp__, as well as a __cmp__. Precedence rules for comparison lookup in Python 2 imply that __cmp__ should never be triggered. The implementation there in some branches will try to look up __cmp__. This would be a good place to start looking into how much we really depend on __cmp__.

comment:7 in reply to: ↑ 5 Changed 7 years ago by jdemeyer

Currently, a Sage Element already implements rich comparison using __cmp__/_cmp_/_cmp_c_impl (for simplification of this, see #17890). In my opinion, there is nothing wrong with keeping that behaviour even in Python 3.

My conclusion from this is that the problem really is cmp() and not __cmp__.

comment:8 Changed 7 years ago by jpflori

  • Cc jpflori added

comment:9 Changed 6 years ago by jdemeyer

  • Component changed from distribution to python3

comment:10 Changed 6 years ago by chapoton

  • Milestone changed from sage-6.4 to sage-7.4
  • Type changed from PLEASE CHANGE to enhancement

comment:11 Changed 5 years ago by chapoton

  • Milestone changed from sage-7.4 to sage-7.6

comment:12 Changed 5 years ago by chapoton

  • Description modified (diff)

comment:13 Changed 5 years ago by chapoton

  • Milestone changed from sage-7.6 to sage-8.0

comment:14 Changed 5 years ago by chapoton

  • Description modified (diff)
  • Milestone changed from sage-8.0 to sage-8.1
Note: See TracTickets for help on using tickets.