diff --git a/cftime/_cftime.pyx b/cftime/_cftime.pyx index f8fd4348..18fbeed0 100644 --- a/cftime/_cftime.pyx +++ b/cftime/_cftime.pyx @@ -2,7 +2,8 @@ Performs conversions of netCDF time coordinate data to/from datetime objects. """ -from cpython.object cimport PyObject_RichCompare +from cpython.object cimport (PyObject_RichCompare, Py_LT, Py_LE, Py_EQ, + Py_NE, Py_GT, Py_GE) import cython import numpy as np import re @@ -37,6 +38,9 @@ cdef int[12] _dpm_360 = [30, 30, 30, 30, 30, 30, 30, 30, 30, 30, 30, 30] # Same as above, but SUM of previous months (no leap years). cdef int[13] _spm_365day = [0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365] cdef int[13] _spm_366day = [0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366] +# Reverse operator lookup for datetime.__richcmp__ +_rop_lookup = {Py_LT: '__gt__', Py_LE: '__ge__', Py_EQ: '__eq__', + Py_GT: '__lt__', Py_GE: '__le__', Py_NE: '__ne__'} __version__ = '1.0.2.1' @@ -1272,16 +1276,29 @@ Gregorial calendar. raise TypeError("cannot compare {0!r} and {1!r} (different calendars)".format(self, other)) return PyObject_RichCompare(dt.to_tuple(), to_tuple(other), op) else: - # With Python3 we can use "return NotImplemented". If the other - # object does not have rich comparison instructions for cftime - # then a TypeError is automatically raised. With Python2 in this - # scenario the default behaviour is to compare the object ids - # which will always have a result. Therefore there is no way to - # differentiate between objects that do or do not have legitimate - # comparisons, and so we cannot remove the TypeError below. - if sys.version_info[0] < 3: - raise TypeError("cannot compare {0!r} and {1!r}".format(self, other)) + # With Python3 we can simply return NotImplemented. If the other + # object does not support rich comparison for cftime then a + # TypeError will be automatically raised. However, Python2 is not + # consistent with this Python3 behaviour. In Python2, we only + # delegate the comparison operation to the other object iff it has + # suitable rich comparison support available. This is deduced by + # introspection of the other object. Otherwise, we explicitly raise + # a TypeError to avoid Python2 defaulting to using either __cmp__ + # comparision on the other object, or worst still, object ID + # comparison. Either way, at this point the comparision is deemed + # not valid from our perspective. + if sys.version_info.major == 2: + rop = _rop_lookup[op] + if (hasattr(other, '__richcmp__') or hasattr(other, rop)): + # The other object potentially has the smarts to handle + # the comparision, so allow the Python machinery to hand + # the operation off to the other object. + return NotImplemented + # Otherwise, the comparison is not valid. + emsg = "cannot compare {0!r} and {1!r}" + raise TypeError(emsg.format(self, other)) else: + # Delegate responsibility of comparison to the other object. return NotImplemented cdef _getstate(self): diff --git a/test/test_cftime.py b/test/test_cftime.py index 14e35116..f2a30da3 100644 --- a/test/test_cftime.py +++ b/test/test_cftime.py @@ -1,5 +1,8 @@ from __future__ import print_function + import copy +import operator +import sys import unittest from collections import namedtuple from datetime import datetime, timedelta @@ -8,13 +11,13 @@ import pytest from numpy.testing import assert_almost_equal, assert_equal +import cftime from cftime import datetime as datetimex from cftime import real_datetime from cftime import (DateFromJulianDay, Datetime360Day, DatetimeAllLeap, DatetimeGregorian, DatetimeJulian, DatetimeNoLeap, DatetimeProlepticGregorian, JulianDayFromDate, _parse_date, date2index, date2num, num2date, utime) -import cftime # test cftime module for netCDF time <--> python datetime conversions. @@ -1106,12 +1109,75 @@ def not_comparable_3(): self.datetime_date1 > self.date2_365_day def not_comparable_4(): - "compare a datetime instance to something other than a datetime" + "compare a datetime instance to non-datetime" self.date1_365_day > 0 + def not_comparable_5(): + "compare non-datetime to a datetime instance" + 0 < self.date_1_365_day + for func in [not_comparable_1, not_comparable_2, not_comparable_3, not_comparable_4]: self.assertRaises(TypeError, func) + @pytest.mark.skipif(sys.version_info.major != 2, + reason='python2 specific, non-comparable test') + def test_richcmp_py2(self): + class Rich(object): + """Dummy class with traditional rich comparison support.""" + def __lt__(self, other): + raise NotImplementedError('__lt__') + def __le__(self, other): + raise NotImplementedError('__le__') + def __eq__(self, other): + raise NotImplementedError('__eq__') + def __ne__(self, other): + raise NotImplementedError('__ne__') + def __gt__(self, other): + raise NotImplementedError('__gt__') + def __ge__(self, other): + raise NotImplementedError('__ge__') + + class CythonRich(object): + """Dummy class with spoof cython rich comparison support.""" + def __richcmp__(self, other): + """ + This method is never called. However it is introspected + by the cftime.datetime.__richcmp__ method, which will then + return NotImplemented, causing Python to call this classes + __cmp__ method as a back-stop, and hence spoofing the + cython specific rich comparison behaviour. + """ + pass + def __cmp__(self, other): + raise NotImplementedError('__richcmp__') + + class Pass(object): + """Dummy class with no rich comparison support whatsoever.""" + pass + + class Pass___cmp__(object): + """Dummy class that delegates all comparisons.""" + def __cmp__(self, other): + return NotImplemented + + # Test LHS operand comparison operator processing. + for op, expected in [(operator.gt, '__lt__'), (operator.ge, '__le__'), + (operator.eq, '__eq__'), (operator.ne, '__ne__'), + (operator.lt, '__gt__'), (operator.le, '__ge__')]: + with self.assertRaisesRegexp(NotImplementedError, expected): + op(self.date1_365_day, Rich()) + + with self.assertRaisesRegexp(NotImplementedError, '__richcmp__'): + op(self.date1_365_day, CythonRich()) + + # Test RHS operand comparison operator processing. + for op in [operator.gt, operator.ge, operator.eq, operator.ne, + operator.lt, operator.le]: + with self.assertRaisesRegexp(TypeError, 'cannot compare'): + op(Pass(), self.date1_365_day) + + with self.assertRaisesRegexp(TypeError, 'cannot compare'): + op(Pass___cmp__(), self.date1_365_day) class issue17TestCase(unittest.TestCase):