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: |
Description (last modified by )
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
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
comment:2 Changed 8 years ago by
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 definedcmp
or using the standard librarycmp
. 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!
- In Py2 "no_cmp" gives often wrong results, but always correct results when
- 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
- Milestone changed from sage-6.3 to sage-6.4
comment:4 Changed 8 years ago by
- Dependencies set to #17175
comment:5 follow-ups: ↓ 6 ↓ 7 Changed 7 years ago by
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
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
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
- Cc jpflori added
comment:9 Changed 6 years ago by
- Component changed from distribution to python3
comment:10 Changed 6 years ago by
- Milestone changed from sage-6.4 to sage-7.4
- Type changed from PLEASE CHANGE to enhancement
comment:11 Changed 5 years ago by
- Milestone changed from sage-7.4 to sage-7.6
comment:12 Changed 5 years ago by
- Description modified (diff)
comment:13 Changed 5 years ago by
- Milestone changed from sage-7.6 to sage-8.0
comment:14 Changed 5 years ago by
- Description modified (diff)
- Milestone changed from sage-8.0 to sage-8.1
I did some grepping to get a first impression of the amount of work required:
git grep -P "def\s+__cmp__" |wc -l
git grep -P "def\s+__eq__" |wc -l
git grep -P "def\s+__ne__" |wc -l
git grep -P "def\s+__lt__" |wc -l
git grep -P "def\s+__le__" |wc -l
git grep -P "def\s+__gt__" |wc -l
git grep -P "def\s+__ge__" |wc -l
git grep -P "\bcmp\s*\(" |wc -l
The conclusion seems that we have to implement rich comparison (i.e. define
__eq__
with__ne__
and__gt__
with the class decoratorfunctools.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!