Skip to content

BUG - remove scaling multiplier from Period diff result #23915

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

Merged
merged 23 commits into from
Dec 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
377f6d1
BUG/TST - do not multiply period diff result by freq scaling factor
sds9995 Nov 26, 2018
b9fe48a
TST - reference issue number in test
sds9995 Nov 26, 2018
c262453
CLN - pep8 adherence
sds9995 Nov 26, 2018
364d4a9
DOC - reference issue in whatsnew
sds9995 Nov 27, 2018
713484c
Revert "DOC - reference issue in whatsnew"
sds9995 Nov 27, 2018
d0a5afe
TST - parameterize test
sds9995 Nov 27, 2018
7ebe407
DOC - reference issue in whatsnew
sds9995 Nov 27, 2018
e8b4c4a
BUG - account for freq kwds
sds9995 Nov 27, 2018
9ad13d8
TST - move non standard freq period diff test and test various keywords
sds9995 Nov 28, 2018
a38ba5a
BUG/CLN - fix periodindex/array diff, and provide base method for off…
sds9995 Dec 2, 2018
cd7bb21
TST/DOC - split tick and offset tests and fix whatsnew
sds9995 Dec 2, 2018
9a369a6
TST - add periodindex diff fix tests
sds9995 Dec 2, 2018
c8f36b9
DOC - add docstrings and GH references
sds9995 Dec 2, 2018
3206144
DOC - additional explanation of code
sds9995 Dec 2, 2018
4b83c3a
Merge branch 'master' into bug/period_diff
sds9995 Dec 4, 2018
1db7bb0
Merge branch 'master' into bug/period_diff
sds9995 Dec 6, 2018
c9a83d6
CLN - reorganize test
sds9995 Dec 6, 2018
7c4e3e6
TST - update tests to account for existing bug
sds9995 Dec 6, 2018
c128a1f
TST - move tick fixture to pandas/conftest
sds9995 Dec 7, 2018
e6d35e6
CLN - fix linting error
sds9995 Dec 7, 2018
ae189ea
DOC - add more detail about this fixs behavior in the whatsnew
sds9995 Dec 7, 2018
8aaf19e
DOC - fix issue reference
sds9995 Dec 7, 2018
9ed3629
DOC - fix spacing
sds9995 Dec 7, 2018
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
4 changes: 3 additions & 1 deletion doc/source/whatsnew/v0.24.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,9 @@ from :class:`Period`, :class:`PeriodIndex`, and in some cases
:class:`Timestamp`, :class:`DatetimeIndex` and :class:`TimedeltaIndex`.

This usage is now deprecated. Instead add or subtract integer multiples of
the object's ``freq`` attribute (:issue:`21939`)
the object's ``freq`` attribute. The result of subtraction of :class:`Period`
objects will be agnostic of the multiplier of the objects' ``freq`` attribute
(:issue:`21939`, :issue:`23878`).

Previous Behavior:

Expand Down
8 changes: 8 additions & 0 deletions pandas/_libs/tslibs/offsets.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,14 @@ class _BaseOffset(object):
if name not in ['n', 'normalize']}
return {name: kwds[name] for name in kwds if kwds[name] is not None}

@property
def base(self):
"""
Returns a copy of the calling offset object with n=1 and all other
attributes equal.
"""
return type(self)(n=1, normalize=self.normalize, **self.kwds)

def __add__(self, other):
if getattr(other, "_typ", None) in ["datetimeindex", "periodindex",
"datetimearray", "periodarray",
Expand Down
3 changes: 2 additions & 1 deletion pandas/_libs/tslibs/period.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1686,7 +1686,8 @@ cdef class _Period(object):
if other.freq != self.freq:
msg = _DIFFERENT_FREQ.format(self.freqstr, other.freqstr)
raise IncompatibleFrequency(msg)
return (self.ordinal - other.ordinal) * self.freq
# GH 23915 - mul by base freq since __add__ is agnostic of n
return (self.ordinal - other.ordinal) * self.freq.base
elif getattr(other, '_typ', None) == 'periodindex':
# GH#21314 PeriodIndex - Period returns an object-index
# of DateOffset objects, for which we cannot use __neg__
Expand Down
8 changes: 8 additions & 0 deletions pandas/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,14 @@ def mock():
return pytest.importorskip("mock")


@pytest.fixture(params=[getattr(pd.offsets, o) for o in pd.offsets.__all__ if
issubclass(getattr(pd.offsets, o), pd.offsets.Tick)])
def tick_classes(request):
"""
Fixture for Tick based datetime offsets available for a time series.
"""
return request.param

# ----------------------------------------------------------------
# Global setup for tests using Hypothesis

Expand Down
2 changes: 1 addition & 1 deletion pandas/core/arrays/datetimelike.py
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ def _sub_period_array(self, other):
arr_mask=self._isnan,
b_mask=other._isnan)

new_values = np.array([self.freq * x for x in new_values])
new_values = np.array([self.freq.base * x for x in new_values])
if self.hasnans or other.hasnans:
mask = (self._isnan) | (other._isnan)
new_values[mask] = NaT
Expand Down
35 changes: 35 additions & 0 deletions pandas/tests/arithmetic/test_period.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,41 @@ def test_parr_sub_pi_mismatched_freq(self, box_with_array):
with pytest.raises(IncompatibleFrequency):
rng - other

@pytest.mark.parametrize('n', [1, 2, 3, 4])
def test_sub_n_gt_1_ticks(self, tick_classes, n):
# GH 23878
p1_d = '19910905'
p2_d = '19920406'
p1 = pd.PeriodIndex([p1_d], freq=tick_classes(n))
p2 = pd.PeriodIndex([p2_d], freq=tick_classes(n))

expected = (pd.PeriodIndex([p2_d], freq=p2.freq.base)
- pd.PeriodIndex([p1_d], freq=p1.freq.base))

tm.assert_index_equal((p2 - p1), expected)

@pytest.mark.parametrize('n', [1, 2, 3, 4])
@pytest.mark.parametrize('offset, kwd_name', [
(pd.offsets.YearEnd, 'month'),
(pd.offsets.QuarterEnd, 'startingMonth'),
(pd.offsets.MonthEnd, None),
(pd.offsets.Week, 'weekday')
])
def test_sub_n_gt_1_offsets(self, offset, kwd_name, n):
# GH 23878
kwds = {kwd_name: 3} if kwd_name is not None else {}
p1_d = '19910905'
p2_d = '19920406'
freq = offset(n, normalize=False, **kwds)
p1 = pd.PeriodIndex([p1_d], freq=freq)
p2 = pd.PeriodIndex([p2_d], freq=freq)

result = p2 - p1
expected = (pd.PeriodIndex([p2_d], freq=freq.base)
- pd.PeriodIndex([p1_d], freq=freq.base))

tm.assert_index_equal(result, expected)

# -------------------------------------------------------------
# Invalid Operations

Expand Down
32 changes: 32 additions & 0 deletions pandas/tests/scalar/period/test_period.py
Original file line number Diff line number Diff line change
Expand Up @@ -1106,6 +1106,38 @@ def test_sub(self):
with pytest.raises(period.IncompatibleFrequency, match=msg):
per1 - Period('2011-02', freq='M')

@pytest.mark.parametrize('n', [1, 2, 3, 4])
def test_sub_n_gt_1_ticks(self, tick_classes, n):
# GH 23878
p1 = pd.Period('19910905', freq=tick_classes(n))
p2 = pd.Period('19920406', freq=tick_classes(n))

expected = (pd.Period(str(p2), freq=p2.freq.base)
- pd.Period(str(p1), freq=p1.freq.base))

assert (p2 - p1) == expected

@pytest.mark.parametrize('normalize', [True, False])
@pytest.mark.parametrize('n', [1, 2, 3, 4])
@pytest.mark.parametrize('offset, kwd_name', [
(pd.offsets.YearEnd, 'month'),
(pd.offsets.QuarterEnd, 'startingMonth'),
(pd.offsets.MonthEnd, None),
(pd.offsets.Week, 'weekday')
])
def test_sub_n_gt_1_offsets(self, offset, kwd_name, n, normalize):
# GH 23878
kwds = {kwd_name: 3} if kwd_name is not None else {}
p1_d = '19910905'
p2_d = '19920406'
p1 = pd.Period(p1_d, freq=offset(n, normalize, **kwds))
p2 = pd.Period(p2_d, freq=offset(n, normalize, **kwds))

expected = (pd.Period(p2_d, freq=p2.freq.base)
- pd.Period(p1_d, freq=p1.freq.base))

assert (p2 - p1) == expected

def test_add_offset(self):
# freq is DateOffset
for freq in ['A', '2A', '3A']:
Expand Down
9 changes: 0 additions & 9 deletions pandas/tests/tseries/offsets/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,3 @@ def month_classes(request):
Fixture for month based datetime offsets available for a time series.
"""
return request.param


@pytest.fixture(params=[getattr(offsets, o) for o in offsets.__all__ if
issubclass(getattr(offsets, o), offsets.Tick)])
def tick_classes(request):
"""
Fixture for Tick based datetime offsets available for a time series.
"""
return request.param