-
-
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 all 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 |
---|---|---|
|
@@ -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.
is there a test that hits this explicity?
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.
Yes. Without this we get test failures in
tests.indexes.period.test_arithmetic.TestPeriodIndexSeriesMethods
fortest_pi_ops
,test_pi_sub_period
,test_pi_sub_period_nat