-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Make Period - Period return DateOffset instead of int #21314
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
Changes from 6 commits
0037273
b981d32
5be05ee
6c1f5f9
3f1b65c
ac7b6b5
108f179
d205afc
5d0b301
1572d6a
3e0ac68
efd7c75
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,10 +26,52 @@ Backwards incompatible API changes | |
|
||
.. _whatsnew_0240.api.datetimelike: | ||
|
||
Period Subtraction | ||
^^^^^^^^^^^^^^^^^^ | ||
|
||
Subtraction of a ``Period`` from another ``Period`` will give a ``DateOffset``. | ||
instead of an integer (:issue:`21314`) | ||
|
||
.. ipython:: python | ||
|
||
june = pd.Period('June 2018') | ||
april = pd.Period('April 2018') | ||
june - april | ||
|
||
Previous Behavior: | ||
|
||
.. code-block:: ipython | ||
|
||
In [2]: june = pd.Period('June 2018') | ||
|
||
In [3]: april = pd.Period('April 2018') | ||
|
||
In [4]: june - april | ||
Out [4]: 2 | ||
|
||
Similarly, subtraction of a ``Period`` from a ``PeriodIndex`` will not return | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will now return |
||
an ``Index`` of ``DateOffset`` objects instead of an ``Int64Index`` | ||
|
||
.. ipython:: python | ||
|
||
pi = pd.period_range('June 2018', freq='M', periods=3) | ||
pi - pi[0] | ||
|
||
Previous Behavior: | ||
|
||
.. code-block:: ipython | ||
|
||
In [2]: pi = pd.period_range('June 2018', freq='M', periods=3) | ||
|
||
In [3]: pi - pi[0] | ||
Out[3]: Int64Index([0, 1, 2], dtype='int64') | ||
|
||
Datetimelike API Changes | ||
^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
- For :class:`DatetimeIndex` and :class:`TimedeltaIndex` with non-``None`` ``freq`` attribute, addition or subtraction of integer-dtyped array or ``Index`` will return an object of the same class (:issue:`19959`) | ||
- Subtraction of :class:`Period` from :class:`Period` will return a :class:`DateOffset` object instead of an integer (:issue:`21314`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you make a new sub-section and show a previous / new behavior example. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "can" part of that is a very good question. I'll try. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. your subsection looks good, you don't need this note here any longer |
||
- Subtraction of :class:`Period` from :class:`PeriodIndex` (or vice-versa) will return an object-dtype :class:`Index` instead of :class:`Int64Index` (:issue:`21314`) | ||
|
||
.. _whatsnew_0240.api.other: | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1120,9 +1120,12 @@ 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 | ||
return (self.ordinal - other.ordinal) * self.freq | ||
elif getattr(other, '_typ', None) == 'periodindex': | ||
return -other.__sub__(self) | ||
# GH#21314 PeriodIndex - Period returns an object-index | ||
# of DateOffset objects, for which we cannot use __neg__ | ||
# directly, so we have to apply it pointwise | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a test that hits this explicity? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Without this we get test failures in |
||
return other.__sub__(self).map(lambda x: -x) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense for the object-dtype Index to attempt |
||
else: # pragma: no cover | ||
return NotImplemented | ||
elif is_period_object(other): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -899,7 +899,9 @@ def __add__(self, other): | |
raise TypeError("cannot add {dtype}-dtype to {cls}" | ||
.format(dtype=other.dtype, | ||
cls=type(self).__name__)) | ||
|
||
elif is_categorical_dtype(other): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can make this is_extension_dtype (here and below) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That should work for now, but it isn't obvious to me how it will shake out as more EA subclasses get implemented. Let me give this some thought. |
||
# Categorical op will raise; defer explicitly | ||
return NotImplemented | ||
else: # pragma: no cover | ||
return NotImplemented | ||
|
||
|
@@ -964,6 +966,9 @@ def __sub__(self, other): | |
raise TypeError("cannot subtract {dtype}-dtype from {cls}" | ||
.format(dtype=other.dtype, | ||
cls=type(self).__name__)) | ||
elif is_categorical_dtype(other): | ||
# Categorical op will raise; defer explicitly | ||
return NotImplemented | ||
else: # pragma: no cover | ||
return NotImplemented | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -551,13 +551,14 @@ def is_all_dates(self): | |
@property | ||
def is_full(self): | ||
""" | ||
Returns True if there are any missing periods from start to end | ||
Returns True if this PeriodIndex is range-like in that all Periods | ||
between start and end are present, in order. | ||
""" | ||
if len(self) == 0: | ||
return True | ||
if not self.is_monotonic: | ||
raise ValueError('Index is not monotonic') | ||
values = self.values | ||
values = self.asi8 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the docstring for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. feel free to update, I don't remember why this was added / what is purpose is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I don't remember even why we have this (can follow up later) |
||
return ((values[1:] - values[:-1]) < 2).all() | ||
|
||
@property | ||
|
@@ -761,17 +762,19 @@ def _sub_datelike(self, other): | |
return NotImplemented | ||
|
||
def _sub_period(self, other): | ||
# If the operation is well-defined, we return an object-Index | ||
# of DateOffsets. Null entries are filled with pd.NaT | ||
if self.freq != other.freq: | ||
msg = _DIFFERENT_FREQ_INDEX.format(self.freqstr, other.freqstr) | ||
raise IncompatibleFrequency(msg) | ||
|
||
asi8 = self.asi8 | ||
new_data = asi8 - other.ordinal | ||
new_data = np.array([self.freq * x for x in new_data]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be nice to implement DateOffset * arraylike so we don't have to right this broadcasting repeatedly. |
||
|
||
if self.hasnans: | ||
new_data = new_data.astype(np.float64) | ||
new_data[self._isnan] = np.nan | ||
# result must be Int64Index or Float64Index | ||
new_data[self._isnan] = tslib.NaT | ||
|
||
return Index(new_data) | ||
|
||
def shift(self, n): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add a reference tag here