Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: PeriodIndex and Period subtraction error #13071

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/source/whatsnew/v0.18.2.txt
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,6 @@ Performance Improvements

Bug Fixes
~~~~~~~~~

- Bug in ``PeriodIndex`` and ``Period`` subtraction raises ``AttributeError`` (:issue:`13071`)
- Bug in ``NaT`` - ``Period`` raises ``AttributeError`` (:issue:`13071`)
44 changes: 27 additions & 17 deletions pandas/src/period.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -826,26 +826,36 @@ cdef class Period(object):
return NotImplemented

def __sub__(self, other):
if isinstance(other, (timedelta, np.timedelta64,
offsets.Tick, offsets.DateOffset, Timedelta)):
neg_other = -other
return self + neg_other
elif lib.is_integer(other):
if self.ordinal == tslib.iNaT:
ordinal = self.ordinal
else:
ordinal = self.ordinal - other * self.freq.n
return Period(ordinal=ordinal, freq=self.freq)
if isinstance(self, Period):
if isinstance(other, (timedelta, np.timedelta64,
offsets.Tick, offsets.DateOffset, Timedelta)):
neg_other = -other
return self + neg_other
elif lib.is_integer(other):
if self.ordinal == tslib.iNaT:
ordinal = self.ordinal
else:
ordinal = self.ordinal - other * self.freq.n
return Period(ordinal=ordinal, freq=self.freq)
elif isinstance(other, Period):
if other.freq != self.freq:
raise ValueError("Cannot do arithmetic with "
"non-conforming periods")
if self.ordinal == tslib.iNaT or other.ordinal == tslib.iNaT:
return Period(ordinal=tslib.iNaT, freq=self.freq)
return self.ordinal - other.ordinal
elif getattr(other, '_typ', None) == 'periodindex':
return -other.__sub__(self)
else: # pragma: no cover
return NotImplemented
elif isinstance(other, Period):
if other.freq != self.freq:
raise ValueError("Cannot do arithmetic with "
"non-conforming periods")
if self.ordinal == tslib.iNaT or other.ordinal == tslib.iNaT:
return Period(ordinal=tslib.iNaT, freq=self.freq)
return self.ordinal - other.ordinal
else: # pragma: no cover
if self is tslib.NaT:
return tslib.NaT
return NotImplemented
else:
return NotImplemented


def asfreq(self, freq, how='E'):
"""
Convert Period to desired frequency, either at the start or end of the
Expand Down
6 changes: 6 additions & 0 deletions pandas/tseries/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
AbstractMethodError)
import pandas.formats.printing as printing
import pandas.tslib as tslib
import pandas._period as _period
import pandas.lib as lib
from pandas.core.index import Index
from pandas.indexes.base import _index_shared_docs
Expand Down Expand Up @@ -533,6 +534,9 @@ def _add_datelike(self, other):
def _sub_datelike(self, other):
raise AbstractMethodError(self)

def _sub_period(self, other):
return NotImplemented

@classmethod
def _add_datetimelike_methods(cls):
"""
Expand Down Expand Up @@ -591,6 +595,8 @@ def __sub__(self, other):
return self.shift(-other)
elif isinstance(other, (tslib.Timestamp, datetime)):
return self._sub_datelike(other)
elif isinstance(other, _period.Period):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whatif Period(...) - pd.NaT ?

Copy link
Member Author

@sinhrks sinhrks May 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NaT will be handled _sub_datelike, currently raises AbstractMethodError.

As in #12759, Period doesn't support NaT and use it's own Period(NaT) ATM.

pd.Period('2011-01', freq='M') - pd.NaT
# NaT

pd.NaT - pd.Period('2011-01', freq='M')
# AttributeError: 'NaTType' object has no attribute 'freq'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this issue cover the 2nd case?

Copy link
Member Author

@sinhrks sinhrks May 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, covered (both Period and PeriodIndex). Added tests.

PeriodIndexP - pd.NaT results in TimedeltaIndex fulfilled with pd.NaT.

return self._sub_period(other)
else: # pragma: no cover
return NotImplemented
cls.__sub__ = __sub__
Expand Down
27 changes: 27 additions & 0 deletions pandas/tseries/period.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pandas.tseries.frequencies as frequencies
from pandas.tseries.frequencies import get_freq_code as _gfc
from pandas.tseries.index import DatetimeIndex, Int64Index, Index
from pandas.tseries.tdi import TimedeltaIndex
from pandas.tseries.base import DatelikeOps, DatetimeIndexOpsMixin
from pandas.tseries.tools import parse_time_string
import pandas.tseries.offsets as offsets
Expand Down Expand Up @@ -595,6 +596,32 @@ def _add_delta(self, other):
ordinal_delta = self._maybe_convert_timedelta(other)
return self.shift(ordinal_delta)

def _sub_datelike(self, other):
if other is tslib.NaT:
new_data = np.empty(len(self), dtype=np.int64)
new_data.fill(tslib.iNaT)
return TimedeltaIndex(new_data, name=self.name)
return NotImplemented

def _sub_period(self, other):
if self.freq != other.freq:
msg = _DIFFERENT_FREQ_INDEX.format(self.freqstr, other.freqstr)
raise IncompatibleFrequency(msg)

if other.ordinal == tslib.iNaT:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this always a Period? see my comment above

Copy link
Member Author

@sinhrks sinhrks May 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always Period. Defined in tseries/base.py.

new_data = np.empty(len(self))
new_data.fill(np.nan)
else:
asi8 = self.asi8
new_data = asi8 - other.ordinal

if self.hasnans:
mask = asi8 == tslib.iNaT
new_data = new_data.astype(np.float64)
new_data[mask] = np.nan
# result must be Int64Index or Float64Index
return Index(new_data, name=self.name)

def shift(self, n):
"""
Specialized shift which produces an PeriodIndex
Expand Down
52 changes: 52 additions & 0 deletions pandas/tseries/tests/test_period.py
Original file line number Diff line number Diff line change
Expand Up @@ -3418,6 +3418,16 @@ def test_add_offset_nat(self):
with tm.assertRaises(ValueError):
p + o

def test_sub_pdnat(self):
# GH 13071
p = pd.Period('2011-01', freq='M')
self.assertIs(p - pd.NaT, pd.NaT)
self.assertIs(pd.NaT - p, pd.NaT)

p = pd.Period('NaT', freq='M')
self.assertIs(p - pd.NaT, pd.NaT)
self.assertIs(pd.NaT - p, pd.NaT)

def test_sub_offset(self):
# freq is DateOffset
for freq in ['A', '2A', '3A']:
Expand Down Expand Up @@ -3614,6 +3624,48 @@ def test_pi_ops_array(self):
'2011-01-01 12:15:00'], freq='S', name='idx')
self.assert_index_equal(result, exp)

def test_pi_sub_period(self):
# GH 13071
idx = PeriodIndex(['2011-01', '2011-02', '2011-03',
'2011-04'], freq='M', name='idx')

result = idx - pd.Period('2012-01', freq='M')
exp = pd.Index([-12, -11, -10, -9], name='idx')
tm.assert_index_equal(result, exp)

result = pd.Period('2012-01', freq='M') - idx
exp = pd.Index([12, 11, 10, 9], name='idx')
tm.assert_index_equal(result, exp)

exp = pd.Index([np.nan, np.nan, np.nan, np.nan], name='idx')
tm.assert_index_equal(idx - pd.Period('NaT', freq='M'), exp)
tm.assert_index_equal(pd.Period('NaT', freq='M') - idx, exp)

def test_pi_sub_pdnat(self):
# GH 13071
idx = PeriodIndex(['2011-01', '2011-02', 'NaT',
'2011-04'], freq='M', name='idx')
exp = pd.TimedeltaIndex([pd.NaT] * 4, name='idx')
tm.assert_index_equal(pd.NaT - idx, exp)
tm.assert_index_equal(idx - pd.NaT, exp)

def test_pi_sub_period_nat(self):
# GH 13071
idx = PeriodIndex(['2011-01', 'NaT', '2011-03',
'2011-04'], freq='M', name='idx')

result = idx - pd.Period('2012-01', freq='M')
exp = pd.Index([-12, np.nan, -10, -9], name='idx')
tm.assert_index_equal(result, exp)

result = pd.Period('2012-01', freq='M') - idx
exp = pd.Index([12, np.nan, 10, 9], name='idx')
tm.assert_index_equal(result, exp)

exp = pd.Index([np.nan, np.nan, np.nan, np.nan], name='idx')
tm.assert_index_equal(idx - pd.Period('NaT', freq='M'), exp)
tm.assert_index_equal(pd.Period('NaT', freq='M') - idx, exp)


class TestPeriodRepresentation(tm.TestCase):
"""
Expand Down