Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1996059

Browse filesBrowse files
gvanrossumgvanrossum
committed
Fix the datetime comparison conundrum.
The special-casing of other objects with a timetuple attribute is gone. Let's hope Tim agrees.
1 parent b6bb0c7 commit 1996059
Copy full SHA for 1996059

File tree

Expand file treeCollapse file tree

2 files changed

+100
-120
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+100
-120
lines changed

‎Lib/test/test_datetime.py

Copy file name to clipboardExpand all lines: Lib/test/test_datetime.py
+53-34Lines changed: 53 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -954,41 +954,60 @@ def test_compare(self):
954954

955955
def test_mixed_compare(self):
956956
our = self.theclass(2000, 4, 5)
957+
958+
# Our class can be compared for equality to other classes
959+
self.assertEqual(our == 1, False)
960+
self.assertEqual(1 == our, False)
961+
self.assertEqual(our != 1, True)
962+
self.assertEqual(1 != our, True)
963+
964+
# But the ordering is undefined
965+
self.assertRaises(TypeError, lambda: our < 1)
966+
self.assertRaises(TypeError, lambda: 1 < our)
957967
self.assertRaises(TypeError, cmp, our, 1)
958968
self.assertRaises(TypeError, cmp, 1, our)
959969

960-
class AnotherDateTimeClass(object):
961-
def __cmp__(self, other):
962-
# Return "equal" so calling this can't be confused with
963-
# compare-by-address (which never says "equal" for distinct
964-
# objects).
965-
return 0
966-
967-
# This still errors, because date and datetime comparison raise
968-
# TypeError instead of NotImplemented when they don't know what to
969-
# do, in order to stop comparison from falling back to the default
970-
# compare-by-address.
971-
their = AnotherDateTimeClass()
970+
# Repeat those tests with a different class
971+
972+
class SomeClass:
973+
pass
974+
975+
their = SomeClass()
976+
self.assertEqual(our == their, False)
977+
self.assertEqual(their == our, False)
978+
self.assertEqual(our != their, True)
979+
self.assertEqual(their != our, True)
980+
self.assertRaises(TypeError, lambda: our < their)
981+
self.assertRaises(TypeError, lambda: their < our)
972982
self.assertRaises(TypeError, cmp, our, their)
973-
# Oops: The next stab raises TypeError in the C implementation,
974-
# but not in the Python implementation of datetime. The difference
975-
# is due to that the Python implementation defines __cmp__ but
976-
# the C implementation defines tp_richcompare. This is more pain
977-
# to fix than it's worth, so commenting out the test.
978-
# self.assertEqual(cmp(their, our), 0)
979-
980-
# But date and datetime comparison return NotImplemented instead if the
981-
# other object has a timetuple attr. This gives the other object a
982-
# chance to do the comparison.
983-
class Comparable(AnotherDateTimeClass):
984-
def timetuple(self):
985-
return ()
986-
987-
their = Comparable()
988-
self.assertEqual(cmp(our, their), 0)
989-
self.assertEqual(cmp(their, our), 0)
990-
self.failUnless(our == their)
991-
self.failUnless(their == our)
983+
self.assertRaises(TypeError, cmp, their, our)
984+
985+
# However, if the other class explicitly defines ordering
986+
# relative to our class, it is allowed to do so
987+
988+
class LargerThanAnything:
989+
def __lt__(self, other):
990+
return False
991+
def __le__(self, other):
992+
return isinstance(other, LargerThanAnything)
993+
def __eq__(self, other):
994+
return isinstance(other, LargerThanAnything)
995+
def __ne__(self, other):
996+
return not isinstance(other, LargerThanAnything)
997+
def __gt__(self, other):
998+
return not isinstance(other, LargerThanAnything)
999+
def __ge__(self, other):
1000+
return True
1001+
1002+
their = LargerThanAnything()
1003+
self.assertEqual(our == their, False)
1004+
self.assertEqual(their == our, False)
1005+
self.assertEqual(our != their, True)
1006+
self.assertEqual(their != our, True)
1007+
self.assertEqual(our < their, True)
1008+
self.assertEqual(their < our, False)
1009+
self.assertEqual(cmp(our, their), -1)
1010+
self.assertEqual(cmp(their, our), 1)
9921011

9931012
def test_bool(self):
9941013
# All dates are considered true.
@@ -3217,10 +3236,10 @@ def test_bug_1028306(self):
32173236

32183237
# Neverthelss, comparison should work with the base-class (date)
32193238
# projection if use of a date method is forced.
3220-
self.assert_(as_date.__eq__(as_datetime))
3239+
self.assertEqual(as_date.__eq__(as_datetime), True)
32213240
different_day = (as_date.day + 1) % 20 + 1
3222-
self.assert_(not as_date.__eq__(as_datetime.replace(day=
3223-
different_day)))
3241+
as_different = as_datetime.replace(day= different_day)
3242+
self.assertEqual(as_date.__eq__(as_different), False)
32243243

32253244
# And date should compare with other subclasses of date. If a
32263245
# subclass wants to stop this, it's up to the subclass to do so.

‎Modules/datetimemodule.c

Copy file name to clipboardExpand all lines: Modules/datetimemodule.c
+47-86Lines changed: 47 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,7 +1387,7 @@ build_struct_time(int y, int m, int d, int hh, int mm, int ss, int dstflag)
13871387
* Miscellaneous helpers.
13881388
*/
13891389

1390-
/* For obscure reasons, we need to use tp_richcompare instead of tp_compare.
1390+
/* For various reasons, we need to use tp_richcompare instead of tp_compare.
13911391
* The comparisons here all most naturally compute a cmp()-like result.
13921392
* This little helper turns that into a bool result for rich comparisons.
13931393
*/
@@ -1705,31 +1705,23 @@ delta_subtract(PyObject *left, PyObject *right)
17051705
return result;
17061706
}
17071707

1708-
/* This is more natural as a tp_compare, but doesn't work then: for whatever
1709-
* reason, Python's try_3way_compare ignores tp_compare unless
1710-
* PyInstance_Check returns true, but these aren't old-style classes.
1711-
*/
17121708
static PyObject *
1713-
delta_richcompare(PyDateTime_Delta *self, PyObject *other, int op)
1709+
delta_richcompare(PyObject *self, PyObject *other, int op)
17141710
{
1715-
int diff = 42; /* nonsense */
1716-
17171711
if (PyDelta_Check(other)) {
1718-
diff = GET_TD_DAYS(self) - GET_TD_DAYS(other);
1712+
int diff = GET_TD_DAYS(self) - GET_TD_DAYS(other);
17191713
if (diff == 0) {
17201714
diff = GET_TD_SECONDS(self) - GET_TD_SECONDS(other);
17211715
if (diff == 0)
17221716
diff = GET_TD_MICROSECONDS(self) -
17231717
GET_TD_MICROSECONDS(other);
17241718
}
1719+
return diff_to_bool(diff, op);
1720+
}
1721+
else {
1722+
Py_INCREF(Py_NotImplemented);
1723+
return Py_NotImplemented;
17251724
}
1726-
else if (op == Py_EQ || op == Py_NE)
1727-
diff = 1; /* any non-zero value will do */
1728-
1729-
else /* stop this from falling back to address comparison */
1730-
return cmperror((PyObject *)self, other);
1731-
1732-
return diff_to_bool(diff, op);
17331725
}
17341726

17351727
static PyObject *delta_getstate(PyDateTime_Delta *self);
@@ -2145,7 +2137,7 @@ static PyTypeObject PyDateTime_DeltaType = {
21452137
delta_doc, /* tp_doc */
21462138
0, /* tp_traverse */
21472139
0, /* tp_clear */
2148-
(richcmpfunc)delta_richcompare, /* tp_richcompare */
2140+
delta_richcompare, /* tp_richcompare */
21492141
0, /* tp_weaklistoffset */
21502142
0, /* tp_iter */
21512143
0, /* tp_iternext */
@@ -2499,31 +2491,19 @@ date_isocalendar(PyDateTime_Date *self)
24992491

25002492
/* Miscellaneous methods. */
25012493

2502-
/* This is more natural as a tp_compare, but doesn't work then: for whatever
2503-
* reason, Python's try_3way_compare ignores tp_compare unless
2504-
* PyInstance_Check returns true, but these aren't old-style classes.
2505-
*/
25062494
static PyObject *
2507-
date_richcompare(PyDateTime_Date *self, PyObject *other, int op)
2495+
date_richcompare(PyObject *self, PyObject *other, int op)
25082496
{
2509-
int diff = 42; /* nonsense */
2510-
2511-
if (PyDate_Check(other))
2512-
diff = memcmp(self->data, ((PyDateTime_Date *)other)->data,
2513-
_PyDateTime_DATE_DATASIZE);
2514-
2515-
else if (PyObject_HasAttrString(other, "timetuple")) {
2516-
/* A hook for other kinds of date objects. */
2497+
if (PyDate_Check(other)) {
2498+
int diff = memcmp(((PyDateTime_Date *)self)->data,
2499+
((PyDateTime_Date *)other)->data,
2500+
_PyDateTime_DATE_DATASIZE);
2501+
return diff_to_bool(diff, op);
2502+
}
2503+
else {
25172504
Py_INCREF(Py_NotImplemented);
25182505
return Py_NotImplemented;
25192506
}
2520-
else if (op == Py_EQ || op == Py_NE)
2521-
diff = 1; /* any non-zero value will do */
2522-
2523-
else /* stop this from falling back to address comparison */
2524-
return cmperror((PyObject *)self, other);
2525-
2526-
return diff_to_bool(diff, op);
25272507
}
25282508

25292509
static PyObject *
@@ -2701,7 +2681,7 @@ static PyTypeObject PyDateTime_DateType = {
27012681
date_doc, /* tp_doc */
27022682
0, /* tp_traverse */
27032683
0, /* tp_clear */
2704-
(richcmpfunc)date_richcompare, /* tp_richcompare */
2684+
date_richcompare, /* tp_richcompare */
27052685
0, /* tp_weaklistoffset */
27062686
0, /* tp_iter */
27072687
0, /* tp_iternext */
@@ -3223,36 +3203,28 @@ time_strftime(PyDateTime_Time *self, PyObject *args, PyObject *kw)
32233203
* Miscellaneous methods.
32243204
*/
32253205

3226-
/* This is more natural as a tp_compare, but doesn't work then: for whatever
3227-
* reason, Python's try_3way_compare ignores tp_compare unless
3228-
* PyInstance_Check returns true, but these aren't old-style classes.
3229-
*/
32303206
static PyObject *
3231-
time_richcompare(PyDateTime_Time *self, PyObject *other, int op)
3207+
time_richcompare(PyObject *self, PyObject *other, int op)
32323208
{
32333209
int diff;
32343210
naivety n1, n2;
32353211
int offset1, offset2;
32363212

32373213
if (! PyTime_Check(other)) {
3238-
if (op == Py_EQ || op == Py_NE) {
3239-
PyObject *result = op == Py_EQ ? Py_False : Py_True;
3240-
Py_INCREF(result);
3241-
return result;
3242-
}
3243-
/* Stop this from falling back to address comparison. */
3244-
return cmperror((PyObject *)self, other);
3214+
Py_INCREF(Py_NotImplemented);
3215+
return Py_NotImplemented;
32453216
}
3246-
if (classify_two_utcoffsets((PyObject *)self, &offset1, &n1, Py_None,
3247-
other, &offset2, &n2, Py_None) < 0)
3217+
if (classify_two_utcoffsets(self, &offset1, &n1, Py_None,
3218+
other, &offset2, &n2, Py_None) < 0)
32483219
return NULL;
32493220
assert(n1 != OFFSET_UNKNOWN && n2 != OFFSET_UNKNOWN);
32503221
/* If they're both naive, or both aware and have the same offsets,
32513222
* we get off cheap. Note that if they're both naive, offset1 ==
32523223
* offset2 == 0 at this point.
32533224
*/
32543225
if (n1 == n2 && offset1 == offset2) {
3255-
diff = memcmp(self->data, ((PyDateTime_Time *)other)->data,
3226+
diff = memcmp(((PyDateTime_Time *)self)->data,
3227+
((PyDateTime_Time *)other)->data,
32563228
_PyDateTime_TIME_DATASIZE);
32573229
return diff_to_bool(diff, op);
32583230
}
@@ -3474,7 +3446,7 @@ static PyTypeObject PyDateTime_TimeType = {
34743446
time_doc, /* tp_doc */
34753447
0, /* tp_traverse */
34763448
0, /* tp_clear */
3477-
(richcmpfunc)time_richcompare, /* tp_richcompare */
3449+
time_richcompare, /* tp_richcompare */
34783450
0, /* tp_weaklistoffset */
34793451
0, /* tp_iter */
34803452
0, /* tp_iternext */
@@ -4115,54 +4087,43 @@ datetime_ctime(PyDateTime_DateTime *self)
41154087

41164088
/* Miscellaneous methods. */
41174089

4118-
/* This is more natural as a tp_compare, but doesn't work then: for whatever
4119-
* reason, Python's try_3way_compare ignores tp_compare unless
4120-
* PyInstance_Check returns true, but these aren't old-style classes.
4121-
*/
41224090
static PyObject *
4123-
datetime_richcompare(PyDateTime_DateTime *self, PyObject *other, int op)
4091+
datetime_richcompare(PyObject *self, PyObject *other, int op)
41244092
{
41254093
int diff;
41264094
naivety n1, n2;
41274095
int offset1, offset2;
41284096

41294097
if (! PyDateTime_Check(other)) {
4130-
/* If other has a "timetuple" attr, that's an advertised
4131-
* hook for other classes to ask to get comparison control.
4132-
* However, date instances have a timetuple attr, and we
4133-
* don't want to allow that comparison. Because datetime
4134-
* is a subclass of date, when mixing date and datetime
4135-
* in a comparison, Python gives datetime the first shot
4136-
* (it's the more specific subtype). So we can stop that
4137-
* combination here reliably.
4138-
*/
4139-
if (PyObject_HasAttrString(other, "timetuple") &&
4140-
! PyDate_Check(other)) {
4141-
/* A hook for other kinds of datetime objects. */
4142-
Py_INCREF(Py_NotImplemented);
4143-
return Py_NotImplemented;
4098+
if (PyDate_Check(other)) {
4099+
/* Prevent invocation of date_richcompare. We want to
4100+
return NotImplemented here to give the other object
4101+
a chance. But since DateTime is a subclass of
4102+
Date, if the other object is a Date, it would
4103+
compute an ordering based on the date part alone,
4104+
and we don't want that. So force unequal or
4105+
uncomparable here in that case. */
4106+
if (op == Py_EQ)
4107+
Py_RETURN_FALSE;
4108+
if (op == Py_NE)
4109+
Py_RETURN_TRUE;
4110+
return cmperror(self, other);
41444111
}
4145-
if (op == Py_EQ || op == Py_NE) {
4146-
PyObject *result = op == Py_EQ ? Py_False : Py_True;
4147-
Py_INCREF(result);
4148-
return result;
4149-
}
4150-
/* Stop this from falling back to address comparison. */
4151-
return cmperror((PyObject *)self, other);
4112+
Py_INCREF(Py_NotImplemented);
4113+
return Py_NotImplemented;
41524114
}
41534115

4154-
if (classify_two_utcoffsets((PyObject *)self, &offset1, &n1,
4155-
(PyObject *)self,
4156-
other, &offset2, &n2,
4157-
other) < 0)
4116+
if (classify_two_utcoffsets(self, &offset1, &n1, self,
4117+
other, &offset2, &n2, other) < 0)
41584118
return NULL;
41594119
assert(n1 != OFFSET_UNKNOWN && n2 != OFFSET_UNKNOWN);
41604120
/* If they're both naive, or both aware and have the same offsets,
41614121
* we get off cheap. Note that if they're both naive, offset1 ==
41624122
* offset2 == 0 at this point.
41634123
*/
41644124
if (n1 == n2 && offset1 == offset2) {
4165-
diff = memcmp(self->data, ((PyDateTime_DateTime *)other)->data,
4125+
diff = memcmp(((PyDateTime_DateTime *)self)->data,
4126+
((PyDateTime_DateTime *)other)->data,
41664127
_PyDateTime_DATETIME_DATASIZE);
41674128
return diff_to_bool(diff, op);
41684129
}
@@ -4568,7 +4529,7 @@ static PyTypeObject PyDateTime_DateTimeType = {
45684529
datetime_doc, /* tp_doc */
45694530
0, /* tp_traverse */
45704531
0, /* tp_clear */
4571-
(richcmpfunc)datetime_richcompare, /* tp_richcompare */
4532+
datetime_richcompare, /* tp_richcompare */
45724533
0, /* tp_weaklistoffset */
45734534
0, /* tp_iter */
45744535
0, /* tp_iternext */

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.