From e7287b366177be48a9030dcdcd88fd872aa35d56 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 30 Jan 2024 21:05:23 +0200 Subject: [PATCH 1/5] gh-49766: Make date-datetime comparison more symmetric and flexible Now the special comparison methods like `__eq__` and `__lt__` return NotImplemented if one of comparands is date and other is datetime instead of ignoring the time part and the time zone or forcefully return "not equal" or raise TypeError. It makes comparison of date and datetime subclasses more symmetric and allows to change the default behavior by overriding the special comparison methods in subclasses. It is now the same as if date and datetime was independent classes. --- Lib/_pydatetime.py | 39 ++++------- Lib/test/datetimetester.py | 64 +++++++++++-------- ...4-01-30-22-10-50.gh-issue-49766.yulJL_.rst | 8 +++ Modules/_datetimemodule.c | 36 +++-------- 4 files changed, 67 insertions(+), 80 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-01-30-22-10-50.gh-issue-49766.yulJL_.rst diff --git a/Lib/_pydatetime.py b/Lib/_pydatetime.py index 355145387e355b..ff979d66a50723 100644 --- a/Lib/_pydatetime.py +++ b/Lib/_pydatetime.py @@ -556,10 +556,6 @@ def _check_tzinfo_arg(tz): if tz is not None and not isinstance(tz, tzinfo): raise TypeError("tzinfo argument must be None or of a tzinfo subclass") -def _cmperror(x, y): - raise TypeError("can't compare '%s' to '%s'" % ( - type(x).__name__, type(y).__name__)) - def _divide_and_round(a, b): """divide a by b and round result to the nearest integer @@ -1113,32 +1109,33 @@ def replace(self, year=None, month=None, day=None): # Comparisons of date objects with other. def __eq__(self, other): - if isinstance(other, date): - return self._cmp(other) == 0 + if isinstance(other, date) and not isinstance(other, datetime): + return self._cmp(other, eq=True) == 0 return NotImplemented def __le__(self, other): - if isinstance(other, date): + if isinstance(other, date) and not isinstance(other, datetime): return self._cmp(other) <= 0 return NotImplemented def __lt__(self, other): - if isinstance(other, date): + if isinstance(other, date) and not isinstance(other, datetime): return self._cmp(other) < 0 return NotImplemented def __ge__(self, other): - if isinstance(other, date): + if isinstance(other, date) and not isinstance(other, datetime): return self._cmp(other) >= 0 return NotImplemented def __gt__(self, other): - if isinstance(other, date): + if isinstance(other, date) and not isinstance(other, datetime): return self._cmp(other) > 0 return NotImplemented - def _cmp(self, other): + def _cmp(self, other, eq=False): assert isinstance(other, date) + assert not isinstance(other, datetime) y, m, d = self._year, self._month, self._day y2, m2, d2 = other._year, other._month, other._day return _cmp((y, m, d), (y2, m2, d2)) @@ -2137,42 +2134,32 @@ def dst(self): def __eq__(self, other): if isinstance(other, datetime): return self._cmp(other, allow_mixed=True) == 0 - elif not isinstance(other, date): - return NotImplemented else: - return False + return NotImplemented def __le__(self, other): if isinstance(other, datetime): return self._cmp(other) <= 0 - elif not isinstance(other, date): - return NotImplemented else: - _cmperror(self, other) + return NotImplemented def __lt__(self, other): if isinstance(other, datetime): return self._cmp(other) < 0 - elif not isinstance(other, date): - return NotImplemented else: - _cmperror(self, other) + return NotImplemented def __ge__(self, other): if isinstance(other, datetime): return self._cmp(other) >= 0 - elif not isinstance(other, date): - return NotImplemented else: - _cmperror(self, other) + return NotImplemented def __gt__(self, other): if isinstance(other, datetime): return self._cmp(other) > 0 - elif not isinstance(other, date): - return NotImplemented else: - _cmperror(self, other) + return NotImplemented def _cmp(self, other, allow_mixed=False): assert isinstance(other, datetime) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 53ad5e57ada017..550f25f58760ee 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -5435,42 +5435,50 @@ def fromutc(self, dt): class Oddballs(unittest.TestCase): - def test_bug_1028306(self): + def test_date_datetime_comparison(self): + # bpo-1028306, bpo-5516 (gh-49766) # Trying to compare a date to a datetime should act like a mixed- # type comparison, despite that datetime is a subclass of date. as_date = date.today() as_datetime = datetime.combine(as_date, time()) - self.assertTrue(as_date != as_datetime) - self.assertTrue(as_datetime != as_date) - self.assertFalse(as_date == as_datetime) - self.assertFalse(as_datetime == as_date) - self.assertRaises(TypeError, lambda: as_date < as_datetime) - self.assertRaises(TypeError, lambda: as_datetime < as_date) - self.assertRaises(TypeError, lambda: as_date <= as_datetime) - self.assertRaises(TypeError, lambda: as_datetime <= as_date) - self.assertRaises(TypeError, lambda: as_date > as_datetime) - self.assertRaises(TypeError, lambda: as_datetime > as_date) - self.assertRaises(TypeError, lambda: as_date >= as_datetime) - self.assertRaises(TypeError, lambda: as_datetime >= as_date) - - # Nevertheless, comparison should work with the base-class (date) - # projection if use of a date method is forced. - self.assertEqual(as_date.__eq__(as_datetime), True) - different_day = (as_date.day + 1) % 20 + 1 - as_different = as_datetime.replace(day= different_day) - self.assertEqual(as_date.__eq__(as_different), False) + date_sc = SubclassDate(as_date.year, as_date.month, as_date.day) + datetime_sc = SubclassDatetime(as_date.year, as_date.month, + as_date.day, 0, 0, 0) + for d in (as_date, date_sc): + for dt in (as_datetime, datetime_sc): + for x, y in (d, dt), (dt, d): + self.assertTrue(x != y) + self.assertFalse(x == y) + self.assertRaises(TypeError, lambda: x < y) + self.assertRaises(TypeError, lambda: x <= y) + self.assertRaises(TypeError, lambda: x > y) + self.assertRaises(TypeError, lambda: x >= y) # And date should compare with other subclasses of date. If a # subclass wants to stop this, it's up to the subclass to do so. - date_sc = SubclassDate(as_date.year, as_date.month, as_date.day) - self.assertEqual(as_date, date_sc) - self.assertEqual(date_sc, as_date) - # Ditto for datetimes. - datetime_sc = SubclassDatetime(as_datetime.year, as_datetime.month, - as_date.day, 0, 0, 0) - self.assertEqual(as_datetime, datetime_sc) - self.assertEqual(datetime_sc, as_datetime) + for x, y in ((as_date, date_sc), + (date_sc, as_date), + (as_datetime, datetime_sc), + (datetime_sc, as_datetime)): + self.assertTrue(x == y) + self.assertFalse(x != y) + self.assertFalse(x < y) + self.assertFalse(x > y) + self.assertTrue(x <= y) + self.assertTrue(x >= y) + + # Nevertheless, comparison should work if other object is an instance + # of date or datetime class with overridden comparison operators. + # So special methods should return NotImplemented, as if + # date and datetime were independent classes. + for x, y in (as_date, as_datetime), (as_datetime, as_date): + self.assertEqual(x.__eq__(y), NotImplemented) + self.assertEqual(x.__ne__(y), NotImplemented) + self.assertEqual(x.__lt__(y), NotImplemented) + self.assertEqual(x.__gt__(y), NotImplemented) + self.assertEqual(x.__gt__(y), NotImplemented) + self.assertEqual(x.__ge__(y), NotImplemented) def test_extra_attributes(self): with self.assertWarns(DeprecationWarning): diff --git a/Misc/NEWS.d/next/Library/2024-01-30-22-10-50.gh-issue-49766.yulJL_.rst b/Misc/NEWS.d/next/Library/2024-01-30-22-10-50.gh-issue-49766.yulJL_.rst new file mode 100644 index 00000000000000..eaaa3ba1cb6f09 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-01-30-22-10-50.gh-issue-49766.yulJL_.rst @@ -0,0 +1,8 @@ +Fix :class:`~datetime.date`-:class:`~datetime.datetime` comparison. Now the +special comparison methods like ``__eq__`` and ``__lt__`` return +:data:`NotImplemented` if one of comparands is :class:`!date` and other is +:class:`!datetime` instead of ignoring the time part and the time zone or +forcefully return "not equal" or raise :exc:`TypeError`. It makes comparison +of :class:`!date` and :class:`!datetime` subclasses more symmetric and +allows to change the default behavior by overriding the special comparison +methods in subclasses. diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index cb5403e8461ff0..1b91341bd0aa77 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -1815,16 +1815,6 @@ diff_to_bool(int diff, int op) Py_RETURN_RICHCOMPARE(diff, 0, op); } -/* Raises a "can't compare" TypeError and returns NULL. */ -static PyObject * -cmperror(PyObject *a, PyObject *b) -{ - PyErr_Format(PyExc_TypeError, - "can't compare %s to %s", - Py_TYPE(a)->tp_name, Py_TYPE(b)->tp_name); - return NULL; -} - /* --------------------------------------------------------------------------- * Class implementations. */ @@ -3447,7 +3437,15 @@ date_isocalendar(PyDateTime_Date *self, PyObject *Py_UNUSED(ignored)) static PyObject * date_richcompare(PyObject *self, PyObject *other, int op) { - if (PyDate_Check(other)) { + /* Since DateTime is a subclass of Date, if the other object is + * a DateTime, it would compute an equality testing or an ordering + * based on the date part alone, and we don't want that. + * So return NotImplemented here in that case. + * If a subclass wants to change this, it's up to the subclass to do so. + * The behavior is the same as if Date and DateTime were independent + * classes. + */ + if (PyDate_Check(other) && !PyDateTime_Check(other)) { int diff = memcmp(((PyDateTime_Date *)self)->data, ((PyDateTime_Date *)other)->data, _PyDateTime_DATE_DATASIZE); @@ -5891,21 +5889,7 @@ datetime_richcompare(PyObject *self, PyObject *other, int op) PyObject *offset1, *offset2; int diff; - if (! PyDateTime_Check(other)) { - if (PyDate_Check(other)) { - /* Prevent invocation of date_richcompare. We want to - return NotImplemented here to give the other object - a chance. But since DateTime is a subclass of - Date, if the other object is a Date, it would - compute an ordering based on the date part alone, - and we don't want that. So force unequal or - uncomparable here in that case. */ - if (op == Py_EQ) - Py_RETURN_FALSE; - if (op == Py_NE) - Py_RETURN_TRUE; - return cmperror(self, other); - } + if (!PyDateTime_Check(other)) { Py_RETURN_NOTIMPLEMENTED; } From f534d9e79529f4750f642708ac1639d3d6004da9 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 2 Feb 2024 22:10:09 +0200 Subject: [PATCH 2/5] Update documentation. --- Doc/library/datetime.rst | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/Doc/library/datetime.rst b/Doc/library/datetime.rst index 096670634ee956..31b831bf26bef2 100644 --- a/Doc/library/datetime.rst +++ b/Doc/library/datetime.rst @@ -619,11 +619,27 @@ Notes: (4) :class:`date` objects are equal if they represent the same date. + :class:`!date` objects that are not also :class:`.datetime` instances + are never equal to :class:`!datetime` objects, even if they represent + the same date. + (5) *date1* is considered less than *date2* when *date1* precedes *date2* in time. In other words, ``date1 < date2`` if and only if ``date1.toordinal() < date2.toordinal()``. + Order comparison between a :class:`!date` object that is not also a + :class:`.datetime` instance and a :class:`!datetime` object raises + :exc:`TypeError`. + +.. versionchanged:: 3.13 + Comparison between :class:`.datetime` object and an instance of + the :class:`date` subclass that is not a :class:`!datetime` subclass + no longer coverts the latter to :class:`!date`, ignoring the time part + and the time zone. + The default behavior can be changed by overriding the special comparison + methods in subclasses. + In Boolean contexts, all :class:`date` objects are considered to be true. Instance methods: @@ -1192,9 +1208,6 @@ Supported operations: and time, taking into account the time zone. Naive and aware :class:`!datetime` objects are never equal. - :class:`!datetime` objects are never equal to :class:`date` objects - that are not also :class:`!datetime` instances, even if they represent - the same date. If both comparands are aware and have different :attr:`~.datetime.tzinfo` attributes, the comparison acts as comparands were first converted to UTC @@ -1206,17 +1219,24 @@ Supported operations: *datetime1* is considered less than *datetime2* when *datetime1* precedes *datetime2* in time, taking into account the time zone. - Order comparison between naive and aware :class:`.datetime` objects, - as well as a :class:`!datetime` object and a :class:`!date` object - that is not also a :class:`!datetime` instance, raises :exc:`TypeError`. + Order comparison between naive and aware :class:`.datetime` objects + raises :exc:`TypeError`. If both comparands are aware and have different :attr:`~.datetime.tzinfo` attributes, the comparison acts as comparands were first converted to UTC datetimes except that the implementation never overflows. - .. versionchanged:: 3.3 - Equality comparisons between aware and naive :class:`.datetime` - instances don't raise :exc:`TypeError`. +.. versionchanged:: 3.3 + Equality comparisons between aware and naive :class:`.datetime` + instances don't raise :exc:`TypeError`. + +.. versionchanged:: 3.13 + Comparison between :class:`.datetime` object and an instance of + the :class:`date` subclass that is not a :class:`!datetime` subclass + no longer coverts the latter to :class:`!date`, ignoring the time part + and the time zone. + The default behavior can be changed by overriding the special comparison + methods in subclasses. Instance methods: From ecc8c4c32492a5d4dc914de772e23ddbd5b64f92 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 2 Feb 2024 22:11:46 +0200 Subject: [PATCH 3/5] Clean up. --- Lib/_pydatetime.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/_pydatetime.py b/Lib/_pydatetime.py index 27f921f9bd8443..15c8bf2c42678b 100644 --- a/Lib/_pydatetime.py +++ b/Lib/_pydatetime.py @@ -1110,7 +1110,7 @@ def replace(self, year=None, month=None, day=None): def __eq__(self, other): if isinstance(other, date) and not isinstance(other, datetime): - return self._cmp(other, eq=True) == 0 + return self._cmp(other) == 0 return NotImplemented def __le__(self, other): @@ -1133,7 +1133,7 @@ def __gt__(self, other): return self._cmp(other) > 0 return NotImplemented - def _cmp(self, other, eq=False): + def _cmp(self, other, eq): assert isinstance(other, date) assert not isinstance(other, datetime) y, m, d = self._year, self._month, self._day From a23a7ef18494204bc55e9ae931320a2201cec06c Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 2 Feb 2024 23:10:00 +0200 Subject: [PATCH 4/5] Fix the clean-up. --- Lib/_pydatetime.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/_pydatetime.py b/Lib/_pydatetime.py index 15c8bf2c42678b..b7d569cc41740e 100644 --- a/Lib/_pydatetime.py +++ b/Lib/_pydatetime.py @@ -1133,7 +1133,7 @@ def __gt__(self, other): return self._cmp(other) > 0 return NotImplemented - def _cmp(self, other, eq): + def _cmp(self, other): assert isinstance(other, date) assert not isinstance(other, datetime) y, m, d = self._year, self._month, self._day From 463252bb7f46e809bf1978e1ff3c761d520c1a0a Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 6 Feb 2024 10:56:25 +0200 Subject: [PATCH 5/5] Normalize indentation for patchcheck. --- Lib/test/datetimetester.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 550f25f58760ee..980a8e6c1b1836 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -5461,12 +5461,12 @@ def test_date_datetime_comparison(self): (date_sc, as_date), (as_datetime, datetime_sc), (datetime_sc, as_datetime)): - self.assertTrue(x == y) - self.assertFalse(x != y) - self.assertFalse(x < y) - self.assertFalse(x > y) - self.assertTrue(x <= y) - self.assertTrue(x >= y) + self.assertTrue(x == y) + self.assertFalse(x != y) + self.assertFalse(x < y) + self.assertFalse(x > y) + self.assertTrue(x <= y) + self.assertTrue(x >= y) # Nevertheless, comparison should work if other object is an instance # of date or datetime class with overridden comparison operators.