Skip to content

Commit 47b9ff6

Browse files
committed
Restructure comparison dramatically. There is no longer a default
*ordering* between objects; there is only a default equality test (defined by an object being equal to itself only). Read the comment in object.c. The current implementation never uses a three-way comparison to compute a rich comparison, but it does use a rich comparison to compute a three-way comparison. I'm not quite done ripping out all the calls to PyObject_Compare/Cmp, or replacing tp_compare implementations with tp_richcompare implementations; but much of that has happened (to make most unit tests pass). The following tests still fail, because I need help deciding or understanding: test_codeop -- depends on comparing code objects test_datetime -- need Tim Peters' opinion test_marshal -- depends on comparing code objects test_mutants -- need help understanding it The problem with test_codeop and test_marshal is this: these tests compare two different code objects and expect them to be equal. Is that still a feature we'd like to support? I've temporarily removed the comparison and hash code from code objects, so they use the default (equality by pointer only) comparison. For the other two tests, run them to see for yourself. (There may be more failing test with "-u all".) A general problem with getting lots of these tests to pass is the reality that for object types that have a natural total ordering, implementing __cmp__ is much more convenient than implementing __eq__, __ne__, __lt__, and so on. Should we go back to allowing __cmp__ to provide a total ordering? Should we provide some other way to implement rich comparison with a single method override? Alex proposed a __key__() method; I've considered a __richcmp__() method. Or perhaps __cmp__() just shouldn't be killed off...
1 parent 9a6e62b commit 47b9ff6

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+969
-899
lines changed

Diff for: Include/object.h

+1
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ PyAPI_FUNC(PyObject *) PyObject_Unicode(PyObject *);
379379
PyAPI_FUNC(int) PyObject_Compare(PyObject *, PyObject *);
380380
PyAPI_FUNC(PyObject *) PyObject_RichCompare(PyObject *, PyObject *, int);
381381
PyAPI_FUNC(int) PyObject_RichCompareBool(PyObject *, PyObject *, int);
382+
PyAPI_FUNC(PyObject *) Py_CmpToRich(int op, int cmp);
382383
PyAPI_FUNC(PyObject *) PyObject_GetAttrString(PyObject *, const char *);
383384
PyAPI_FUNC(int) PyObject_SetAttrString(PyObject *, const char *, PyObject *);
384385
PyAPI_FUNC(int) PyObject_HasAttrString(PyObject *, const char *);

Diff for: Lib/StringIO.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ def tell(self):
116116
_complain_ifclosed(self.closed)
117117
return self.pos
118118

119-
def read(self, n = -1):
119+
def read(self, n=None):
120120
"""Read at most size bytes from the file
121121
(less if the read hits EOF before obtaining size bytes).
122122
@@ -128,6 +128,8 @@ def read(self, n = -1):
128128
if self.buflist:
129129
self.buf += ''.join(self.buflist)
130130
self.buflist = []
131+
if n is None:
132+
n = -1
131133
if n < 0:
132134
newpos = self.len
133135
else:

Diff for: Lib/UserDict.py

+14-7
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,16 @@ def __init__(self, dict=None, **kwargs):
88
if len(kwargs):
99
self.update(kwargs)
1010
def __repr__(self): return repr(self.data)
11-
def __cmp__(self, dict):
11+
def __eq__(self, dict):
1212
if isinstance(dict, UserDict):
13-
return cmp(self.data, dict.data)
13+
return self.data == dict.data
1414
else:
15-
return cmp(self.data, dict)
15+
return self.data == dict
16+
def __ne__(self, dict):
17+
if isinstance(dict, UserDict):
18+
return self.data != dict.data
19+
else:
20+
return self.data != dict
1621
def __len__(self): return len(self.data)
1722
def __getitem__(self, key):
1823
if key in self.data:
@@ -162,11 +167,13 @@ def get(self, key, default=None):
162167
return default
163168
def __repr__(self):
164169
return repr(dict(self.iteritems()))
165-
def __cmp__(self, other):
166-
if other is None:
167-
return 1
170+
def __eq__(self, other):
171+
if isinstance(other, DictMixin):
172+
other = dict(other.iteritems())
173+
return dict(self.iteritems()) == other
174+
def __ne__(self, other):
168175
if isinstance(other, DictMixin):
169176
other = dict(other.iteritems())
170-
return cmp(dict(self.iteritems()), other)
177+
return dict(self.iteritems()) != other
171178
def __len__(self):
172179
return len(self.keys())

Diff for: Lib/UserString.py

+29-3
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,37 @@ def __float__(self): return float(self.data)
2525
def __complex__(self): return complex(self.data)
2626
def __hash__(self): return hash(self.data)
2727

28-
def __cmp__(self, string):
28+
def __eq__(self, string):
2929
if isinstance(string, UserString):
30-
return cmp(self.data, string.data)
30+
return self.data == string.data
3131
else:
32-
return cmp(self.data, string)
32+
return self.data == string
33+
def __ne__(self, string):
34+
if isinstance(string, UserString):
35+
return self.data != string.data
36+
else:
37+
return self.data != string
38+
def __lt__(self, string):
39+
if isinstance(string, UserString):
40+
return self.data < string.data
41+
else:
42+
return self.data < string
43+
def __le__(self, string):
44+
if isinstance(string, UserString):
45+
return self.data <= string.data
46+
else:
47+
return self.data <= string
48+
def __gt__(self, string):
49+
if isinstance(string, UserString):
50+
return self.data > string.data
51+
else:
52+
return self.data > string
53+
def __ge__(self, string):
54+
if isinstance(string, UserString):
55+
return self.data >= string.data
56+
else:
57+
return self.data >= string
58+
3359
def __contains__(self, char):
3460
return char in self.data
3561

Diff for: Lib/ctypes/test/test_simplesubclasses.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22
from ctypes import *
33

44
class MyInt(c_int):
5-
def __cmp__(self, other):
5+
def __eq__(self, other):
66
if type(other) != MyInt:
7-
return -1
8-
return cmp(self.value, other.value)
7+
return NotImplementedError
8+
return self.value == other.value
99

1010
class Test(unittest.TestCase):
1111

Diff for: Lib/decimal.py

+21
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,26 @@ def __ne__(self, other):
706706
return NotImplemented
707707
return self.__cmp__(other) != 0
708708

709+
def __lt__(self, other):
710+
if not isinstance(other, (Decimal, int, long)):
711+
return NotImplemented
712+
return self.__cmp__(other) < 0
713+
714+
def __le__(self, other):
715+
if not isinstance(other, (Decimal, int, long)):
716+
return NotImplemented
717+
return self.__cmp__(other) <= 0
718+
719+
def __gt__(self, other):
720+
if not isinstance(other, (Decimal, int, long)):
721+
return NotImplemented
722+
return self.__cmp__(other) > 0
723+
724+
def __ge__(self, other):
725+
if not isinstance(other, (Decimal, int, long)):
726+
return NotImplemented
727+
return self.__cmp__(other) >= 0
728+
709729
def compare(self, other, context=None):
710730
"""Compares one to another.
711731
@@ -1894,6 +1914,7 @@ def to_integral(self, rounding=None, context=None):
18941914
ans = self._check_nans(context=context)
18951915
if ans:
18961916
return ans
1917+
return self
18971918
if self._exp >= 0:
18981919
return self
18991920
if context is None:

Diff for: Lib/distutils/version.py

+38-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@
3232
class Version:
3333
"""Abstract base class for version numbering classes. Just provides
3434
constructor (__init__) and reproducer (__repr__), because those
35-
seem to be the same for all version numbering classes.
35+
seem to be the same for all version numbering classes; and route
36+
rich comparisons to __cmp__.
3637
"""
3738

3839
def __init__ (self, vstring=None):
@@ -42,6 +43,42 @@ def __init__ (self, vstring=None):
4243
def __repr__ (self):
4344
return "%s ('%s')" % (self.__class__.__name__, str(self))
4445

46+
def __eq__(self, other):
47+
c = self.__cmp__(other)
48+
if c is NotImplemented:
49+
return c
50+
return c == 0
51+
52+
def __ne__(self, other):
53+
c = self.__cmp__(other)
54+
if c is NotImplemented:
55+
return c
56+
return c != 0
57+
58+
def __lt__(self, other):
59+
c = self.__cmp__(other)
60+
if c is NotImplemented:
61+
return c
62+
return c < 0
63+
64+
def __le__(self, other):
65+
c = self.__cmp__(other)
66+
if c is NotImplemented:
67+
return c
68+
return c <= 0
69+
70+
def __gt__(self, other):
71+
c = self.__cmp__(other)
72+
if c is NotImplemented:
73+
return c
74+
return c > 0
75+
76+
def __ge__(self, other):
77+
c = self.__cmp__(other)
78+
if c is NotImplemented:
79+
return c
80+
return c >= 0
81+
4582

4683
# Interface for version-number classes -- must be implemented
4784
# by the following classes (the concrete ones -- Version should

Diff for: Lib/doctest.py

+5-4
Original file line numberDiff line numberDiff line change
@@ -469,11 +469,12 @@ def __repr__(self):
469469

470470

471471
# This lets us sort tests by name:
472-
def __cmp__(self, other):
472+
def __lt__(self, other):
473473
if not isinstance(other, DocTest):
474-
return -1
475-
return cmp((self.name, self.filename, self.lineno, id(self)),
476-
(other.name, other.filename, other.lineno, id(other)))
474+
return NotImplemented
475+
return ((self.name, self.filename, self.lineno, id(self))
476+
<
477+
(other.name, other.filename, other.lineno, id(other)))
477478

478479
######################################################################
479480
## 3. DocTestParser

Diff for: Lib/optparse.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -838,13 +838,13 @@ def __str__(self):
838838

839839
__repr__ = _repr
840840

841-
def __cmp__(self, other):
841+
def __eq__(self, other):
842842
if isinstance(other, Values):
843-
return cmp(self.__dict__, other.__dict__)
843+
return self.__dict__ == other.__dict__
844844
elif isinstance(other, types.DictType):
845-
return cmp(self.__dict__, other)
845+
return self.__dict__ == other
846846
else:
847-
return -1
847+
return NotImplemented
848848

849849
def _update_careful(self, dict):
850850
"""

Diff for: Lib/plat-mac/plistlib.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -374,13 +374,13 @@ def fromBase64(cls, data):
374374
def asBase64(self, maxlinelength=76):
375375
return _encodeBase64(self.data, maxlinelength)
376376

377-
def __cmp__(self, other):
377+
def __eq__(self, other):
378378
if isinstance(other, self.__class__):
379-
return cmp(self.data, other.data)
379+
return self.data == other.data
380380
elif isinstance(other, str):
381-
return cmp(self.data, other)
381+
return self.data == other
382382
else:
383-
return cmp(id(self), id(other))
383+
return id(self) == id(other)
384384

385385
def __repr__(self):
386386
return "%s(%s)" % (self.__class__.__name__, repr(self.data))

Diff for: Lib/pprint.py

+6-1
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,12 @@ def _safe_repr(object, context, maxlevels, level):
246246
append = components.append
247247
level += 1
248248
saferepr = _safe_repr
249-
for k, v in sorted(object.items()):
249+
items = object.items()
250+
try:
251+
items = sorted(items)
252+
except TypeError:
253+
items = sorted(items, key=lambda (k, v): (str(type(k)), k, v))
254+
for k, v in items:
250255
krepr, kreadable, krecur = saferepr(k, context, maxlevels, level)
251256
vrepr, vreadable, vrecur = saferepr(v, context, maxlevels, level)
252257
append("%s: %s" % (krepr, vrepr))

Diff for: Lib/sqlite3/test/types.py

+6
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ def __cmp__(self, other):
8686
else:
8787
return 1
8888

89+
def __eq__(self, other):
90+
c = self.__cmp__(other)
91+
if c is NotImplemented:
92+
return c
93+
return c == 0
94+
8995
def __conform__(self, protocol):
9096
if protocol is sqlite.PrepareProtocol:
9197
return self.val

Diff for: Lib/test/mapping_tests.py

+11-14
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,10 @@ def test_read(self):
6060
for k in self.other:
6161
self.failIf(k in d)
6262
#cmp
63-
self.assertEqual(cmp(p,p), 0)
64-
self.assertEqual(cmp(d,d), 0)
65-
self.assertEqual(cmp(p,d), -1)
66-
self.assertEqual(cmp(d,p), 1)
63+
self.assertEqual(p, p)
64+
self.assertEqual(d, d)
65+
self.assertNotEqual(p, d)
66+
self.assertNotEqual(d, p)
6767
#__non__zero__
6868
if p: self.fail("Empty mapping must compare to False")
6969
if not d: self.fail("Full mapping must compare to True")
@@ -623,26 +623,23 @@ def __repr__(self):
623623
d = self._full_mapping({1: BadRepr()})
624624
self.assertRaises(Exc, repr, d)
625625

626-
def test_le(self):
627-
self.assert_(not (self._empty_mapping() < self._empty_mapping()))
628-
self.assert_(not (self._full_mapping({1: 2}) < self._full_mapping({1L: 2L})))
626+
def test_eq(self):
627+
self.assertEqual(self._empty_mapping(), self._empty_mapping())
628+
self.assertEqual(self._full_mapping({1: 2}),
629+
self._full_mapping({1L: 2L}))
629630

630631
class Exc(Exception): pass
631632

632633
class BadCmp(object):
633634
def __eq__(self, other):
634635
raise Exc()
635636
def __hash__(self):
636-
return 42
637+
return 1
637638

638639
d1 = self._full_mapping({BadCmp(): 1})
639640
d2 = self._full_mapping({1: 1})
640-
try:
641-
d1 < d2
642-
except Exc:
643-
pass
644-
else:
645-
self.fail("< didn't raise Exc")
641+
self.assertRaises(Exc, lambda: BadCmp()==1)
642+
self.assertRaises(Exc, lambda: d1==d2)
646643

647644
def test_setdefault(self):
648645
TestMappingProtocol.test_setdefault(self)

Diff for: Lib/test/output/test_class

+10-10
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,16 @@ __hex__: ()
5151
__hash__: ()
5252
__repr__: ()
5353
__str__: ()
54-
__cmp__: (1,)
55-
__cmp__: (1,)
56-
__cmp__: (1,)
57-
__cmp__: (1,)
58-
__cmp__: (1,)
59-
__cmp__: (1,)
60-
__cmp__: (1,)
61-
__cmp__: (1,)
62-
__cmp__: (1,)
63-
__cmp__: (1,)
54+
__eq__: (1,)
55+
__lt__: (1,)
56+
__gt__: (1,)
57+
__ne__: (1,)
58+
__ne__: (1,)
59+
__eq__: (1,)
60+
__gt__: (1,)
61+
__lt__: (1,)
62+
__ne__: (1,)
63+
__ne__: (1,)
6464
__del__: ()
6565
__getattr__: ('spam',)
6666
__setattr__: ('eggs', 'spam, spam, spam and ham')

Diff for: Lib/test/pickletester.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ def restore(self):
6565
copy_reg.add_extension(pair[0], pair[1], code)
6666

6767
class C:
68-
def __cmp__(self, other):
69-
return cmp(self.__dict__, other.__dict__)
68+
def __eq__(self, other):
69+
return self.__dict__ == other.__dict__
7070

7171
import __main__
7272
__main__.C = C

Diff for: Lib/test/test_bisect.py

+6-1
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,13 @@ def __getitem__(self, ndx):
174174

175175
class CmpErr:
176176
"Dummy element that always raises an error during comparison"
177-
def __cmp__(self, other):
177+
def __lt__(self, other):
178178
raise ZeroDivisionError
179+
__gt__ = __lt__
180+
__le__ = __lt__
181+
__ge__ = __lt__
182+
__eq__ = __lt__
183+
__ne__ = __lt__
179184

180185
class TestErrorHandling(unittest.TestCase):
181186

0 commit comments

Comments
 (0)