From 02911870be755b1e44cfdd1979ad6c0646f1c8b8 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Mon, 18 May 2020 20:08:50 -0700 Subject: [PATCH 1/2] BUG: NaT+Period doesnt match Period+NaT --- pandas/_libs/tslibs/period.pyx | 96 +++++++++++------------ pandas/core/arrays/period.py | 4 +- pandas/tests/scalar/period/test_period.py | 13 ++- 3 files changed, 58 insertions(+), 55 deletions(-) diff --git a/pandas/_libs/tslibs/period.pyx b/pandas/_libs/tslibs/period.pyx index 246abd0602add..d28585f15e5b5 100644 --- a/pandas/_libs/tslibs/period.pyx +++ b/pandas/_libs/tslibs/period.pyx @@ -52,7 +52,6 @@ from pandas._libs.tslibs.ccalendar cimport ( from pandas._libs.tslibs.ccalendar cimport c_MONTH_NUMBERS from pandas._libs.tslibs.frequencies cimport ( attrname_to_abbrevs, - get_base_alias, get_freq_code, get_freq_str, get_rule_month, @@ -1600,9 +1599,7 @@ cdef class _Period: raise IncompatibleFrequency("Input cannot be converted to " f"Period(freq={self.freqstr})") elif util.is_offset_object(other): - freqstr = other.rule_code - base = get_base_alias(freqstr) - if base == self.freq.rule_code: + if other.base == self.freq.base: ordinal = self.ordinal + other.n return Period(ordinal=ordinal, freq=self.freq) msg = DIFFERENT_FREQ.format(cls=type(self).__name__, @@ -1613,58 +1610,57 @@ cdef class _Period: return NotImplemented def __add__(self, other): - if is_period_object(self): - if (PyDelta_Check(other) or util.is_timedelta64_object(other) or - util.is_offset_object(other)): - return self._add_delta(other) - elif other is NaT: + if not is_period_object(self): + # cython semantics; this is analogous to a call to __radd__ + if self is NaT: return NaT - elif util.is_integer_object(other): - ordinal = self.ordinal + other * self.freq.n - return Period(ordinal=ordinal, freq=self.freq) - elif (PyDateTime_Check(other) or - is_period_object(other) or util.is_datetime64_object(other)): - # can't add datetime-like - # GH#17983 - sname = type(self).__name__ - oname = type(other).__name__ - raise TypeError(f"unsupported operand type(s) for +: '{sname}' " - f"and '{oname}'") - else: # pragma: no cover - return NotImplemented - elif is_period_object(other): - # this can be reached via __radd__ because of cython rules - return other + self - else: - return NotImplemented + return other.__add__(self) + + if (PyDelta_Check(other) or util.is_timedelta64_object(other) or + util.is_offset_object(other)): + return self._add_delta(other) + elif other is NaT: + return NaT + elif util.is_integer_object(other): + ordinal = self.ordinal + other * self.freq.n + return Period(ordinal=ordinal, freq=self.freq) + elif (PyDateTime_Check(other) or + is_period_object(other) or util.is_datetime64_object(other)): + # can't add datetime-like + # GH#17983 + sname = type(self).__name__ + oname = type(other).__name__ + raise TypeError(f"unsupported operand type(s) for +: '{sname}' " + f"and '{oname}'") + + return NotImplemented def __sub__(self, other): - if is_period_object(self): - if (PyDelta_Check(other) or util.is_timedelta64_object(other) or - util.is_offset_object(other)): - neg_other = -other - return self + neg_other - elif util.is_integer_object(other): - ordinal = self.ordinal - other * self.freq.n - return Period(ordinal=ordinal, freq=self.freq) - elif is_period_object(other): - if other.freq != self.freq: - msg = DIFFERENT_FREQ.format(cls=type(self).__name__, - own_freq=self.freqstr, - other_freq=other.freqstr) - raise IncompatibleFrequency(msg) - # GH 23915 - mul by base freq since __add__ is agnostic of n - return (self.ordinal - other.ordinal) * self.freq.base - elif other is NaT: - return NaT - return NotImplemented - elif is_period_object(other): - # this can be reached via __rsub__ because of cython rules + if not is_period_object(self): + # cython semantics; this is like a call to __rsub__ if self is NaT: return NaT return NotImplemented - else: - return NotImplemented + + elif (PyDelta_Check(other) or util.is_timedelta64_object(other) or + util.is_offset_object(other)): + neg_other = -other + return self + neg_other + elif util.is_integer_object(other): + ordinal = self.ordinal - other * self.freq.n + return Period(ordinal=ordinal, freq=self.freq) + elif is_period_object(other): + if other.freq != self.freq: + msg = DIFFERENT_FREQ.format(cls=type(self).__name__, + own_freq=self.freqstr, + other_freq=other.freqstr) + raise IncompatibleFrequency(msg) + # GH 23915 - mul by base freq since __add__ is agnostic of n + return (self.ordinal - other.ordinal) * self.freq.base + elif other is NaT: + return NaT + + return NotImplemented def asfreq(self, freq, how='E') -> "Period": """ diff --git a/pandas/core/arrays/period.py b/pandas/core/arrays/period.py index 7a7fdba88cda1..f1f8abb9e93a2 100644 --- a/pandas/core/arrays/period.py +++ b/pandas/core/arrays/period.py @@ -667,8 +667,8 @@ def _addsub_int_array( def _add_offset(self, other): assert not isinstance(other, Tick) - base = libfrequencies.get_base_alias(other.rule_code) - if base != self.freq.rule_code: + + if other.base != self.freq.base: raise raise_on_incompatible(self, other) # Note: when calling parent class's _add_timedeltalike_scalar, diff --git a/pandas/tests/scalar/period/test_period.py b/pandas/tests/scalar/period/test_period.py index 01b61da099481..1480980f5f772 100644 --- a/pandas/tests/scalar/period/test_period.py +++ b/pandas/tests/scalar/period/test_period.py @@ -1061,7 +1061,7 @@ def test_add_invalid(self): per1 = Period(freq="D", year=2008, month=1, day=1) per2 = Period(freq="D", year=2008, month=1, day=2) - msg = r"unsupported operand type\(s\)" + msg = r"unsupported operand type\(s\)|can only concatenate str" with pytest.raises(TypeError, match=msg): per1 + "str" with pytest.raises(TypeError, match=msg): @@ -1402,8 +1402,15 @@ def test_sub_offset(self): @pytest.mark.parametrize("freq", ["M", "2M", "3M"]) def test_period_addsub_nat(self, freq): - assert NaT - Period("2011-01", freq=freq) is NaT - assert Period("2011-01", freq=freq) - NaT is NaT + per = Period("2011-01", freq=freq) + + # For subtraction, NaT is treated as another Period object + assert NaT - per is NaT + assert per - NaT is NaT + + # For addition, NaT is treated as offset-like + assert NaT + per is NaT + assert per + NaT is NaT def test_period_ops_offset(self): p = Period("2011-04-01", freq="D") From d3683a21910067d97418ed71b599639053b0dd9f Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Mon, 18 May 2020 20:46:48 -0700 Subject: [PATCH 2/2] version compat --- pandas/tests/scalar/period/test_period.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pandas/tests/scalar/period/test_period.py b/pandas/tests/scalar/period/test_period.py index 1480980f5f772..e81f2ee55eebd 100644 --- a/pandas/tests/scalar/period/test_period.py +++ b/pandas/tests/scalar/period/test_period.py @@ -1061,7 +1061,13 @@ def test_add_invalid(self): per1 = Period(freq="D", year=2008, month=1, day=1) per2 = Period(freq="D", year=2008, month=1, day=2) - msg = r"unsupported operand type\(s\)|can only concatenate str" + msg = "|".join( + [ + r"unsupported operand type\(s\)", + "can only concatenate str", + "must be str, not Period", + ] + ) with pytest.raises(TypeError, match=msg): per1 + "str" with pytest.raises(TypeError, match=msg):