-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Fix name setting in DTI/TDI __add__ and __sub__ #19744
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 9 commits
6bb94fe
191b2fb
21a5864
0b51e00
205b09c
27ed3bb
d67d8e5
9bb2f9a
a10a9fb
34b2514
e78d361
249b7ba
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 |
---|---|---|
|
@@ -886,7 +886,7 @@ def _sub_datelike(self, other): | |
else: | ||
raise TypeError("cannot subtract DatetimeIndex and {typ}" | ||
.format(typ=type(other).__name__)) | ||
return TimedeltaIndex(result, name=self.name, copy=False) | ||
return TimedeltaIndex(result) | ||
|
||
def _sub_datelike_dti(self, other): | ||
"""subtraction of two DatetimeIndexes""" | ||
|
@@ -910,28 +910,39 @@ def _maybe_update_attributes(self, attrs): | |
return attrs | ||
|
||
def _add_delta(self, delta): | ||
if isinstance(delta, ABCSeries): | ||
return NotImplemented | ||
""" | ||
Add a timedelta-like, DateOffset, or TimedeltaIndex-like object | ||
to self. | ||
|
||
Parameters | ||
---------- | ||
delta : {timedelta, np.timedelta64, DateOffset, | ||
TimedelaIndex, ndarray[timedelta64]} | ||
|
||
Returns | ||
------- | ||
result : DatetimeIndex | ||
|
||
Notes | ||
----- | ||
The result's name is set outside of _add_delta by the calling | ||
method (__add__ or __sub__) | ||
""" | ||
from pandas import TimedeltaIndex | ||
name = self.name | ||
|
||
if isinstance(delta, (Tick, timedelta, np.timedelta64)): | ||
new_values = self._add_delta_td(delta) | ||
elif is_timedelta64_dtype(delta): | ||
if not isinstance(delta, TimedeltaIndex): | ||
delta = TimedeltaIndex(delta) | ||
else: | ||
# update name when delta is Index | ||
name = com._maybe_match_name(self, delta) | ||
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. let's move _maybe_match_name to ops and put next to get_op_result_name (and should remove it in favor of get_op_result_name, but that might be slightly tricky). 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. OK. While I'm at it I'm going to move get_op_result_name with the utility functions near the top of the file instead of in the arithmetic-specific spot it is now. |
||
new_values = self._add_delta_tdi(delta) | ||
elif isinstance(delta, DateOffset): | ||
new_values = self._add_offset(delta).asi8 | ||
else: | ||
new_values = self.astype('O') + delta | ||
|
||
tz = 'UTC' if self.tz is not None else None | ||
result = DatetimeIndex(new_values, tz=tz, name=name, freq='infer') | ||
result = DatetimeIndex(new_values, tz=tz, freq='infer') | ||
if self.tz is not None and self.tz is not utc: | ||
result = result.tz_convert(self.tz) | ||
return result | ||
|
@@ -954,22 +965,19 @@ def _add_offset(self, offset): | |
|
||
def _add_offset_array(self, other): | ||
# Array/Index of DateOffset objects | ||
if isinstance(other, ABCSeries): | ||
return NotImplemented | ||
elif len(other) == 1: | ||
if len(other) == 1: | ||
return self + other[0] | ||
else: | ||
warnings.warn("Adding/subtracting array of DateOffsets to " | ||
"{} not vectorized".format(type(self)), | ||
PerformanceWarning) | ||
return self.astype('O') + np.array(other) | ||
# TODO: pass freq='infer' like we do in _sub_offset_array? | ||
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. what is the purpose of these comments (esp here, after the 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. a) to make sure it gets seen during review, b) as a note to myself or the next pass |
||
# TODO: This works for __add__ but loses dtype in __sub__ | ||
|
||
def _sub_offset_array(self, other): | ||
# Array/Index of DateOffset objects | ||
if isinstance(other, ABCSeries): | ||
return NotImplemented | ||
elif len(other) == 1: | ||
if len(other) == 1: | ||
return self - other[0] | ||
else: | ||
warnings.warn("Adding/subtracting array of DateOffsets to " | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -356,19 +356,32 @@ def _maybe_update_attributes(self, attrs): | |
return attrs | ||
|
||
def _add_delta(self, delta): | ||
""" | ||
Add a timedelta-like, Tick, or TimedeltaIndex-like object | ||
to self. | ||
|
||
Parameters | ||
---------- | ||
delta : {timedelta, np.timedelta64, Tick, TimedeltaIndex} | ||
|
||
Returns | ||
------- | ||
result : TimedeltaIndex | ||
|
||
Notes | ||
----- | ||
The result's name is set outside of _add_delta by the calling | ||
method (__add__ or __sub__) | ||
""" | ||
if isinstance(delta, (Tick, timedelta, np.timedelta64)): | ||
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. should prob add a doc-string to these also making the guarantee that the name should not be set |
||
new_values = self._add_delta_td(delta) | ||
name = self.name | ||
elif isinstance(delta, TimedeltaIndex): | ||
new_values = self._add_delta_tdi(delta) | ||
# update name when delta is index | ||
name = com._maybe_match_name(self, delta) | ||
else: | ||
raise TypeError("cannot add the type {0} to a TimedeltaIndex" | ||
.format(type(delta))) | ||
|
||
result = TimedeltaIndex(new_values, freq='infer', name=name) | ||
return result | ||
return TimedeltaIndex(new_values, freq='infer') | ||
|
||
def _evaluate_with_timedelta_like(self, other, op, opstr, reversed=False): | ||
if isinstance(other, ABCSeries): | ||
|
@@ -409,7 +422,7 @@ def _add_datelike(self, other): | |
result = checked_add_with_arr(i8, other.value, | ||
arr_mask=self._isnan) | ||
result = self._maybe_mask_results(result, fill_value=iNaT) | ||
return DatetimeIndex(result, name=self.name, copy=False) | ||
return DatetimeIndex(result) | ||
|
||
def _sub_datelike(self, other): | ||
# GH#19124 Timedelta - datetime is not in general well-defined. | ||
|
@@ -426,16 +439,15 @@ def _add_offset_array(self, other): | |
# TimedeltaIndex can only operate with a subset of DateOffset | ||
# subclasses. Incompatible classes will raise AttributeError, | ||
# which we re-raise as TypeError | ||
if isinstance(other, ABCSeries): | ||
return NotImplemented | ||
elif len(other) == 1: | ||
if len(other) == 1: | ||
return self + other[0] | ||
else: | ||
from pandas.errors import PerformanceWarning | ||
warnings.warn("Adding/subtracting array of DateOffsets to " | ||
"{} not vectorized".format(type(self)), | ||
PerformanceWarning) | ||
return self.astype('O') + np.array(other) | ||
# TODO: pass freq='infer' like we do in _sub_offset_array? | ||
# TODO: This works for __add__ but loses dtype in __sub__ | ||
except AttributeError: | ||
raise TypeError("Cannot add non-tick DateOffset to TimedeltaIndex") | ||
|
@@ -446,9 +458,7 @@ def _sub_offset_array(self, other): | |
# TimedeltaIndex can only operate with a subset of DateOffset | ||
# subclasses. Incompatible classes will raise AttributeError, | ||
# which we re-raise as TypeError | ||
if isinstance(other, ABCSeries): | ||
return NotImplemented | ||
elif len(other) == 1: | ||
if len(other) == 1: | ||
return self - other[0] | ||
else: | ||
from pandas.errors import PerformanceWarning | ||
|
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.
simply set
.name
(and same above)