From 6bb94fe18dfff1aa8838459860d04a80212fecd7 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sat, 17 Feb 2018 14:57:34 -0800 Subject: [PATCH 1/9] Delay name setting to ensure we do it consistently --- pandas/core/indexes/datetimelike.py | 51 ++++++++----- pandas/core/indexes/datetimes.py | 20 ++--- pandas/core/indexes/period.py | 4 +- pandas/core/indexes/timedeltas.py | 17 ++--- .../indexes/datetimes/test_arithmetic.py | 44 +++++++++-- .../indexes/timedeltas/test_arithmetic.py | 75 ++++++++++++------- 6 files changed, 129 insertions(+), 82 deletions(-) diff --git a/pandas/core/indexes/datetimelike.py b/pandas/core/indexes/datetimelike.py index c98f8ceea0ffa..036ef32c29fe6 100644 --- a/pandas/core/indexes/datetimelike.py +++ b/pandas/core/indexes/datetimelike.py @@ -29,7 +29,7 @@ from pandas.core.dtypes.generic import ( ABCIndex, ABCSeries, ABCPeriodIndex, ABCIndexClass) from pandas.core.dtypes.missing import isna -from pandas.core import common as com, algorithms +from pandas.core import common as com, algorithms, ops from pandas.core.algorithms import checked_add_with_arr from pandas.errors import NullFrequencyError import pandas.io.formats.printing as printing @@ -661,29 +661,37 @@ def __add__(self, other): if isinstance(other, ABCSeries): return NotImplemented elif is_timedelta64_dtype(other): - return self._add_delta(other) + result = self._add_delta(other) elif isinstance(other, (DateOffset, timedelta)): - return self._add_delta(other) + result = self._add_delta(other) elif is_offsetlike(other): # Array/Index of DateOffset objects - return self._add_offset_array(other) + result = self._add_offset_array(other) elif isinstance(self, TimedeltaIndex) and isinstance(other, Index): if hasattr(other, '_add_delta'): - return other._add_delta(self) - raise TypeError("cannot add TimedeltaIndex and {typ}" - .format(typ=type(other))) + result = other._add_delta(self) + else: + raise TypeError("cannot add TimedeltaIndex and {typ}" + .format(typ=type(other))) elif is_integer(other): - return self.shift(other) + # This check must come after the check for timedelta64_dtype + # or else it will incorrectly catch np.timedelta64 objects + result = self.shift(other) elif isinstance(other, (datetime, np.datetime64)): - return self._add_datelike(other) + result = self._add_datelike(other) elif isinstance(other, Index): - return self._add_datelike(other) + result = self._add_datelike(other) elif is_integer_dtype(other) and self.freq is None: # GH#19123 raise NullFrequencyError("Cannot shift with no freq") else: # pragma: no cover return NotImplemented + if result is not NotImplemented: + res_name = ops._get_series_op_result_name(self, other) + result = result.rename(name=res_name) + return result + cls.__add__ = __add__ cls.__radd__ = __add__ @@ -697,25 +705,27 @@ def __sub__(self, other): if isinstance(other, ABCSeries): return NotImplemented elif is_timedelta64_dtype(other): - return self._add_delta(-other) + result = self._add_delta(-other) elif isinstance(other, (DateOffset, timedelta)): - return self._add_delta(-other) + result = self._add_delta(-other) elif is_offsetlike(other): # Array/Index of DateOffset objects - return self._sub_offset_array(other) + result = self._sub_offset_array(other) elif isinstance(self, TimedeltaIndex) and isinstance(other, Index): if not isinstance(other, TimedeltaIndex): raise TypeError("cannot subtract TimedeltaIndex and {typ}" .format(typ=type(other).__name__)) - return self._add_delta(-other) + result = self._add_delta(-other) elif isinstance(other, DatetimeIndex): - return self._sub_datelike(other) + result = self._sub_datelike(other) elif is_integer(other): - return self.shift(-other) + # This check must come after the check for timedelta64_dtype + # or else it will incorrectly catch np.timedelta64 objects + result = self.shift(-other) elif isinstance(other, (datetime, np.datetime64)): - return self._sub_datelike(other) + result = self._sub_datelike(other) elif isinstance(other, Period): - return self._sub_period(other) + result = self._sub_period(other) elif isinstance(other, Index): raise TypeError("cannot subtract {typ1} and {typ2}" .format(typ1=type(self).__name__, @@ -726,6 +736,11 @@ def __sub__(self, other): else: # pragma: no cover return NotImplemented + if result is not NotImplemented: + res_name = ops._get_series_op_result_name(self, other) + result = result.rename(name=res_name) + return result + cls.__sub__ = __sub__ def __rsub__(self, other): diff --git a/pandas/core/indexes/datetimes.py b/pandas/core/indexes/datetimes.py index cc9ce1f3fd5eb..7f47b339450ad 100644 --- a/pandas/core/indexes/datetimes.py +++ b/pandas/core/indexes/datetimes.py @@ -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,20 +910,13 @@ def _maybe_update_attributes(self, attrs): return attrs def _add_delta(self, delta): - if isinstance(delta, ABCSeries): - return NotImplemented - 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) new_values = self._add_delta_tdi(delta) elif isinstance(delta, DateOffset): new_values = self._add_offset(delta).asi8 @@ -931,7 +924,7 @@ def _add_delta(self, delta): 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 +947,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? # 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 " diff --git a/pandas/core/indexes/period.py b/pandas/core/indexes/period.py index 8f2d7d382a16e..60798e6d77e37 100644 --- a/pandas/core/indexes/period.py +++ b/pandas/core/indexes/period.py @@ -729,7 +729,7 @@ 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 TimedeltaIndex(new_data) return NotImplemented def _sub_period(self, other): @@ -744,7 +744,7 @@ def _sub_period(self, other): new_data = new_data.astype(np.float64) new_data[self._isnan] = np.nan # result must be Int64Index or Float64Index - return Index(new_data, name=self.name) + return Index(new_data) def shift(self, n): """ diff --git a/pandas/core/indexes/timedeltas.py b/pandas/core/indexes/timedeltas.py index 4b543262fc485..726665a48b24a 100644 --- a/pandas/core/indexes/timedeltas.py +++ b/pandas/core/indexes/timedeltas.py @@ -358,17 +358,13 @@ def _maybe_update_attributes(self, attrs): def _add_delta(self, delta): if isinstance(delta, (Tick, timedelta, np.timedelta64)): 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 +405,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,9 +422,7 @@ 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 @@ -436,6 +430,7 @@ def _add_offset_array(self, other): "{} 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 +441,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 diff --git a/pandas/tests/indexes/datetimes/test_arithmetic.py b/pandas/tests/indexes/datetimes/test_arithmetic.py index ddc97636ae0a8..b726bef09b7ac 100644 --- a/pandas/tests/indexes/datetimes/test_arithmetic.py +++ b/pandas/tests/indexes/datetimes/test_arithmetic.py @@ -721,11 +721,10 @@ def test_dti_add_series(self, tz, names): result4 = index + ser.values tm.assert_index_equal(result4, expected) - @pytest.mark.parametrize('box', [np.array, pd.Index]) - def test_dti_add_offset_array(self, tz, box): + def test_dti_add_offset_array(self, tz): # GH#18849 dti = pd.date_range('2017-01-01', periods=2, tz=tz) - other = box([pd.offsets.MonthEnd(), pd.offsets.Day(n=2)]) + other = np.array([pd.offsets.MonthEnd(), pd.offsets.Day(n=2)]) with tm.assert_produces_warning(PerformanceWarning): res = dti + other @@ -737,11 +736,29 @@ def test_dti_add_offset_array(self, tz, box): res2 = other + dti tm.assert_index_equal(res2, expected) - @pytest.mark.parametrize('box', [np.array, pd.Index]) - def test_dti_sub_offset_array(self, tz, box): + @pytest.mark.parametrize('names', [(None, None, None), + ('foo', 'bar', None), + ('foo', 'foo', 'foo')]) + def test_dti_add_offset_index(self, tz, names): + # GH#18849 + dti = pd.date_range('2017-01-01', periods=2, tz=tz, name=names[0]) + other = pd.Index([pd.offsets.MonthEnd(), pd.offsets.Day(n=2)], + name=names[1]) + + with tm.assert_produces_warning(PerformanceWarning): + res = dti + other + expected = DatetimeIndex([dti[n] + other[n] for n in range(len(dti))], + name=names[2], freq='infer') + tm.assert_index_equal(res, expected) + + with tm.assert_produces_warning(PerformanceWarning): + res2 = other + dti + tm.assert_index_equal(res2, expected) + + def test_dti_sub_offset_array(self, tz): # GH#18824 dti = pd.date_range('2017-01-01', periods=2, tz=tz) - other = box([pd.offsets.MonthEnd(), pd.offsets.Day(n=2)]) + other = np.array([pd.offsets.MonthEnd(), pd.offsets.Day(n=2)]) with tm.assert_produces_warning(PerformanceWarning): res = dti - other @@ -749,6 +766,21 @@ def test_dti_sub_offset_array(self, tz, box): name=dti.name, freq='infer') tm.assert_index_equal(res, expected) + @pytest.mark.parametrize('names', [(None, None, None), + ('foo', 'bar', None), + ('foo', 'foo', 'foo')]) + def test_dti_sub_offset_index(self, tz, names): + # GH#18824 + dti = pd.date_range('2017-01-01', periods=2, tz=tz, name=names[0]) + other = pd.Index([pd.offsets.MonthEnd(), pd.offsets.Day(n=2)], + name=names[1]) + + with tm.assert_produces_warning(PerformanceWarning): + res = dti - other + expected = DatetimeIndex([dti[n] - other[n] for n in range(len(dti))], + name=names[2], freq='infer') + tm.assert_index_equal(res, expected) + @pytest.mark.parametrize('names', [(None, None, None), ('foo', 'bar', None), ('foo', 'foo', 'foo')]) diff --git a/pandas/tests/indexes/timedeltas/test_arithmetic.py b/pandas/tests/indexes/timedeltas/test_arithmetic.py index 3dc60ed33b958..bd37cbfab733d 100644 --- a/pandas/tests/indexes/timedeltas/test_arithmetic.py +++ b/pandas/tests/indexes/timedeltas/test_arithmetic.py @@ -194,11 +194,31 @@ def test_shift_no_freq(self): # ------------------------------------------------------------- - @pytest.mark.parametrize('box', [np.array, pd.Index]) - def test_tdi_add_offset_array(self, box): + @pytest.mark.parametrize('names', [(None, None, None), + ('foo', 'bar', None), + ('foo', 'foo', 'foo')]) + def test_tdi_add_offset_index(self, names): + # GH#18849 + tdi = TimedeltaIndex(['1 days 00:00:00', '3 days 04:00:00'], + name=names[0]) + other = pd.Index([pd.offsets.Hour(n=1), pd.offsets.Minute(n=-2)], + name=names[1]) + + expected = TimedeltaIndex([tdi[n] + other[n] for n in range(len(tdi))], + freq='infer', name=names[2]) + + with tm.assert_produces_warning(PerformanceWarning): + res = tdi + other + tm.assert_index_equal(res, expected) + + with tm.assert_produces_warning(PerformanceWarning): + res2 = other + tdi + tm.assert_index_equal(res2, expected) + + def test_tdi_add_offset_array(self): # GH#18849 tdi = TimedeltaIndex(['1 days 00:00:00', '3 days 04:00:00']) - other = box([pd.offsets.Hour(n=1), pd.offsets.Minute(n=-2)]) + other = np.array([pd.offsets.Hour(n=1), pd.offsets.Minute(n=-2)]) expected = TimedeltaIndex([tdi[n] + other[n] for n in range(len(tdi))], freq='infer') @@ -211,23 +231,27 @@ def test_tdi_add_offset_array(self, box): res2 = other + tdi tm.assert_index_equal(res2, expected) - anchored = box([pd.offsets.QuarterEnd(), - pd.offsets.Week(weekday=2)]) + @pytest.mark.parametrize('names', [(None, None, None), + ('foo', 'bar', None), + ('foo', 'foo', 'foo')]) + def test_tdi_sub_offset_index(self, names): + # GH#18824 + tdi = TimedeltaIndex(['1 days 00:00:00', '3 days 04:00:00'], + name=names[0]) + other = pd.Index([pd.offsets.Hour(n=1), pd.offsets.Minute(n=-2)], + name=names[1]) - # addition/subtraction ops with anchored offsets should issue - # a PerformanceWarning and _then_ raise a TypeError. - with pytest.raises(TypeError): - with tm.assert_produces_warning(PerformanceWarning): - tdi + anchored - with pytest.raises(TypeError): - with tm.assert_produces_warning(PerformanceWarning): - anchored + tdi + expected = TimedeltaIndex([tdi[n] - other[n] for n in range(len(tdi))], + freq='infer', name=names[2]) - @pytest.mark.parametrize('box', [np.array, pd.Index]) - def test_tdi_sub_offset_array(self, box): + with tm.assert_produces_warning(PerformanceWarning): + res = tdi - other + tm.assert_index_equal(res, expected) + + def test_tdi_sub_offset_array(self): # GH#18824 tdi = TimedeltaIndex(['1 days 00:00:00', '3 days 04:00:00']) - other = box([pd.offsets.Hour(n=1), pd.offsets.Minute(n=-2)]) + other = np.array([pd.offsets.Hour(n=1), pd.offsets.Minute(n=-2)]) expected = TimedeltaIndex([tdi[n] - other[n] for n in range(len(tdi))], freq='infer') @@ -236,17 +260,6 @@ def test_tdi_sub_offset_array(self, box): res = tdi - other tm.assert_index_equal(res, expected) - anchored = box([pd.offsets.MonthEnd(), pd.offsets.Day(n=2)]) - - # addition/subtraction ops with anchored offsets should issue - # a PerformanceWarning and _then_ raise a TypeError. - with pytest.raises(TypeError): - with tm.assert_produces_warning(PerformanceWarning): - tdi - anchored - with pytest.raises(TypeError): - with tm.assert_produces_warning(PerformanceWarning): - anchored - tdi - @pytest.mark.parametrize('names', [(None, None, None), ('foo', 'bar', None), ('foo', 'foo', 'foo')]) @@ -275,8 +288,12 @@ def test_tdi_with_offset_series(self, names): res3 = tdi - other tm.assert_series_equal(res3, expected_sub) - anchored = Series([pd.offsets.MonthEnd(), pd.offsets.Day(n=2)], - name=names[1]) + @pytest.mark.parametrize('box', [np.array, pd.Index, pd.Series]) + def test_tdi_add_sub_anchored_offset_arraylike(self, box): + # GH#18824 + tdi = TimedeltaIndex(['1 days 00:00:00', '3 days 04:00:00']) + + anchored = box([pd.offsets.MonthEnd(), pd.offsets.Day(n=2)]) # addition/subtraction ops with anchored offsets should issue # a PerformanceWarning and _then_ raise a TypeError. From 191b2fb350da1a388a339c2801099fc83300775d Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sat, 17 Feb 2018 15:00:27 -0800 Subject: [PATCH 2/9] Whatsnew note --- doc/source/whatsnew/v0.23.0.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/source/whatsnew/v0.23.0.txt b/doc/source/whatsnew/v0.23.0.txt index a2198d9103528..765526653de7d 100644 --- a/doc/source/whatsnew/v0.23.0.txt +++ b/doc/source/whatsnew/v0.23.0.txt @@ -716,6 +716,7 @@ Datetimelike - Bug in :class:`Timestamp` and :func:`to_datetime` where a string representing a barely out-of-bounds timestamp would be incorrectly rounded down instead of raising ``OutOfBoundsDatetime`` (:issue:`19382`) - Bug in :func:`Timestamp.floor` :func:`DatetimeIndex.floor` where time stamps far in the future and past were not rounded correctly (:issue:`19206`) - Bug in :func:`to_datetime` where passing an out-of-bounds datetime with ``errors='coerce'`` and ``utc=True`` would raise ``OutOfBoundsDatetime`` instead of parsing to ``NaT`` (:issue:`19612`) +- Bug in :class:`DatetimeIndex` and :class:`TimedeltaIndex` addition and subtraction where name of the returned object was not always set consistently. (:issue:``) - Timezones From 21a5864b3cf4686f46c16e27343df2bccd79441e Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sat, 17 Feb 2018 15:02:12 -0800 Subject: [PATCH 3/9] update GH reference --- doc/source/whatsnew/v0.23.0.txt | 2 +- pandas/tests/indexes/datetimes/test_arithmetic.py | 4 ++-- pandas/tests/indexes/timedeltas/test_arithmetic.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/source/whatsnew/v0.23.0.txt b/doc/source/whatsnew/v0.23.0.txt index 765526653de7d..2d031cb7f3325 100644 --- a/doc/source/whatsnew/v0.23.0.txt +++ b/doc/source/whatsnew/v0.23.0.txt @@ -716,7 +716,7 @@ Datetimelike - Bug in :class:`Timestamp` and :func:`to_datetime` where a string representing a barely out-of-bounds timestamp would be incorrectly rounded down instead of raising ``OutOfBoundsDatetime`` (:issue:`19382`) - Bug in :func:`Timestamp.floor` :func:`DatetimeIndex.floor` where time stamps far in the future and past were not rounded correctly (:issue:`19206`) - Bug in :func:`to_datetime` where passing an out-of-bounds datetime with ``errors='coerce'`` and ``utc=True`` would raise ``OutOfBoundsDatetime`` instead of parsing to ``NaT`` (:issue:`19612`) -- Bug in :class:`DatetimeIndex` and :class:`TimedeltaIndex` addition and subtraction where name of the returned object was not always set consistently. (:issue:``) +- Bug in :class:`DatetimeIndex` and :class:`TimedeltaIndex` addition and subtraction where name of the returned object was not always set consistently. (:issue:`19744`) - Timezones diff --git a/pandas/tests/indexes/datetimes/test_arithmetic.py b/pandas/tests/indexes/datetimes/test_arithmetic.py index b726bef09b7ac..f252d6ec31f89 100644 --- a/pandas/tests/indexes/datetimes/test_arithmetic.py +++ b/pandas/tests/indexes/datetimes/test_arithmetic.py @@ -740,7 +740,7 @@ def test_dti_add_offset_array(self, tz): ('foo', 'bar', None), ('foo', 'foo', 'foo')]) def test_dti_add_offset_index(self, tz, names): - # GH#18849 + # GH#18849, GH#19744 dti = pd.date_range('2017-01-01', periods=2, tz=tz, name=names[0]) other = pd.Index([pd.offsets.MonthEnd(), pd.offsets.Day(n=2)], name=names[1]) @@ -770,7 +770,7 @@ def test_dti_sub_offset_array(self, tz): ('foo', 'bar', None), ('foo', 'foo', 'foo')]) def test_dti_sub_offset_index(self, tz, names): - # GH#18824 + # GH#18824, GH#19744 dti = pd.date_range('2017-01-01', periods=2, tz=tz, name=names[0]) other = pd.Index([pd.offsets.MonthEnd(), pd.offsets.Day(n=2)], name=names[1]) diff --git a/pandas/tests/indexes/timedeltas/test_arithmetic.py b/pandas/tests/indexes/timedeltas/test_arithmetic.py index bd37cbfab733d..029fdfcefc299 100644 --- a/pandas/tests/indexes/timedeltas/test_arithmetic.py +++ b/pandas/tests/indexes/timedeltas/test_arithmetic.py @@ -198,7 +198,7 @@ def test_shift_no_freq(self): ('foo', 'bar', None), ('foo', 'foo', 'foo')]) def test_tdi_add_offset_index(self, names): - # GH#18849 + # GH#18849, GH#19744 tdi = TimedeltaIndex(['1 days 00:00:00', '3 days 04:00:00'], name=names[0]) other = pd.Index([pd.offsets.Hour(n=1), pd.offsets.Minute(n=-2)], @@ -235,7 +235,7 @@ def test_tdi_add_offset_array(self): ('foo', 'bar', None), ('foo', 'foo', 'foo')]) def test_tdi_sub_offset_index(self, names): - # GH#18824 + # GH#18824, GH#19744 tdi = TimedeltaIndex(['1 days 00:00:00', '3 days 04:00:00'], name=names[0]) other = pd.Index([pd.offsets.Hour(n=1), pd.offsets.Minute(n=-2)], From 0b51e00af703ebce32f4534be6dac9d716b1984c Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sun, 18 Feb 2018 11:17:45 -0800 Subject: [PATCH 4/9] docstrings, rename get_series_op_result_name-->get_op_result_name --- pandas/core/indexes/datetimelike.py | 8 +++---- pandas/core/indexes/datetimes.py | 18 ++++++++++++++ pandas/core/indexes/timedeltas.py | 17 +++++++++++++ pandas/core/ops.py | 37 +++++++++++++++++++++-------- 4 files changed, 66 insertions(+), 14 deletions(-) diff --git a/pandas/core/indexes/datetimelike.py b/pandas/core/indexes/datetimelike.py index 036ef32c29fe6..ecda88057d4ae 100644 --- a/pandas/core/indexes/datetimelike.py +++ b/pandas/core/indexes/datetimelike.py @@ -688,8 +688,8 @@ def __add__(self, other): return NotImplemented if result is not NotImplemented: - res_name = ops._get_series_op_result_name(self, other) - result = result.rename(name=res_name) + res_name = ops.get_op_result_name(self, other) + result = result.rename(name=res_name, inplace=True) return result cls.__add__ = __add__ @@ -737,8 +737,8 @@ def __sub__(self, other): return NotImplemented if result is not NotImplemented: - res_name = ops._get_series_op_result_name(self, other) - result = result.rename(name=res_name) + res_name = ops.get_op_result_name(self, other) + result = result.rename(name=res_name, inplace=True) return result cls.__sub__ = __sub__ diff --git a/pandas/core/indexes/datetimes.py b/pandas/core/indexes/datetimes.py index 7f47b339450ad..debeabf9bae23 100644 --- a/pandas/core/indexes/datetimes.py +++ b/pandas/core/indexes/datetimes.py @@ -910,6 +910,24 @@ def _maybe_update_attributes(self, attrs): return attrs def _add_delta(self, delta): + """ + 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 if isinstance(delta, (Tick, timedelta, np.timedelta64)): diff --git a/pandas/core/indexes/timedeltas.py b/pandas/core/indexes/timedeltas.py index 726665a48b24a..af1c2f131fd60 100644 --- a/pandas/core/indexes/timedeltas.py +++ b/pandas/core/indexes/timedeltas.py @@ -356,6 +356,23 @@ 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)): new_values = self._add_delta_td(delta) elif isinstance(delta, TimedeltaIndex): diff --git a/pandas/core/ops.py b/pandas/core/ops.py index 4c234ccb4dd47..7f319ac479d5a 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -42,6 +42,32 @@ ABCSparseSeries, ABCSparseArray) +# ----------------------------------------------------------------------------- +# Ops Wrapping Utilities + +def get_op_result_name(left, right): + """ + Find the appropriate name to pin to an operation result. This result + should always be either an Index or a Series. + + Parameters + ---------- + left : {Series, Index} + right : object + + Returns + ------- + name : object + Usually a string + """ + # `left` is always a pd.Series when called from within ops + if isinstance(right, (ABCSeries, pd.Index)): + name = com._maybe_match_name(left, right) + else: + name = left.name + return name + + # ----------------------------------------------------------------------------- # Reversed Operations not available in the stdlib operator module. # Defining these instead of using lambdas allows us to reference them by name. @@ -747,7 +773,7 @@ def wrapper(left, right, name=name, na_op=na_op): return NotImplemented left, right = _align_method_SERIES(left, right) - res_name = _get_series_op_result_name(left, right) + res_name = get_op_result_name(left, right) if is_datetime64_dtype(left) or is_datetime64tz_dtype(left): result = dispatch_to_index_op(op, left, right, pd.DatetimeIndex) @@ -811,15 +837,6 @@ def dispatch_to_index_op(op, left, right, index_class): return result -def _get_series_op_result_name(left, right): - # `left` is always a pd.Series - if isinstance(right, (ABCSeries, pd.Index)): - name = com._maybe_match_name(left, right) - else: - name = left.name - return name - - def _comp_method_OBJECT_ARRAY(op, x, y): if isinstance(y, list): y = construct_1d_object_array_from_listlike(y) From 27ed3bbfaea3c709343803a0d33182ac1dde34bb Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sun, 18 Feb 2018 11:20:21 -0800 Subject: [PATCH 5/9] move maybe_match_name from com to ops --- pandas/core/common.py | 15 --------------- pandas/core/ops.py | 21 ++++++++++++++++++--- pandas/core/series.py | 6 +++--- pandas/tests/test_common.py | 13 +++++++------ 4 files changed, 28 insertions(+), 27 deletions(-) diff --git a/pandas/core/common.py b/pandas/core/common.py index 6748db825acf0..77dc1522052d4 100644 --- a/pandas/core/common.py +++ b/pandas/core/common.py @@ -121,21 +121,6 @@ def _consensus_name_attr(objs): return name -def _maybe_match_name(a, b): - a_has = hasattr(a, 'name') - b_has = hasattr(b, 'name') - if a_has and b_has: - if a.name == b.name: - return a.name - else: - return None - elif a_has: - return a.name - elif b_has: - return b.name - return None - - def _get_info_slice(obj, indexer): """Slice the info axis of `obj` with `indexer`.""" if not hasattr(obj, '_info_axis_number'): diff --git a/pandas/core/ops.py b/pandas/core/ops.py index 8c8b7e06ed98f..9208d8fcfcf14 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -62,12 +62,27 @@ def get_op_result_name(left, right): """ # `left` is always a pd.Series when called from within ops if isinstance(right, (ABCSeries, pd.Index)): - name = com._maybe_match_name(left, right) + name = _maybe_match_name(left, right) else: name = left.name return name +def _maybe_match_name(a, b): + a_has = hasattr(a, 'name') + b_has = hasattr(b, 'name') + if a_has and b_has: + if a.name == b.name: + return a.name + else: + return None + elif a_has: + return a.name + elif b_has: + return b.name + return None + + # ----------------------------------------------------------------------------- # Reversed Operations not available in the stdlib operator module. # Defining these instead of using lambdas allows us to reference them by name. @@ -1059,7 +1074,7 @@ def wrapper(self, other): return NotImplemented elif isinstance(other, ABCSeries): - name = com._maybe_match_name(self, other) + name = _maybe_match_name(self, other) is_other_int_dtype = is_integer_dtype(other.dtype) other = fill_int(other) if is_other_int_dtype else fill_bool(other) @@ -1485,7 +1500,7 @@ def wrapper(self, other): def _sparse_series_op(left, right, op, name): left, right = left.align(right, join='outer', copy=False) new_index = left.index - new_name = com._maybe_match_name(left, right) + new_name = _maybe_match_name(left, right) from pandas.core.sparse.array import _sparse_array_op result = _sparse_array_op(left.values, right.values, op, name, diff --git a/pandas/core/series.py b/pandas/core/series.py index 90dc14836ab55..4e10dd3f9f5d1 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -1728,7 +1728,7 @@ def _binop(self, other, func, level=None, fill_value=None): with np.errstate(all='ignore'): result = func(this_vals, other_vals) - name = com._maybe_match_name(self, other) + name = ops._maybe_match_name(self, other) result = self._constructor(result, index=new_index, name=name) result = result.__finalize__(self) if name is None: @@ -1769,7 +1769,7 @@ def combine(self, other, func, fill_value=np.nan): """ if isinstance(other, Series): new_index = self.index.union(other.index) - new_name = com._maybe_match_name(self, other) + new_name = ops._maybe_match_name(self, other) new_values = np.empty(len(new_index), dtype=self.dtype) for i, idx in enumerate(new_index): lv = self.get(idx, fill_value) @@ -1814,7 +1814,7 @@ def combine_first(self, other): this = self.reindex(new_index, copy=False) other = other.reindex(new_index, copy=False) # TODO: do we need name? - name = com._maybe_match_name(self, other) # noqa + name = ops._maybe_match_name(self, other) # noqa rs_vals = com._where_compat(isna(this), other._values, this._values) return self._constructor(rs_vals, index=new_index).__finalize__(self) diff --git a/pandas/tests/test_common.py b/pandas/tests/test_common.py index 57479be4d989f..0b329f64dafa3 100644 --- a/pandas/tests/test_common.py +++ b/pandas/tests/test_common.py @@ -9,6 +9,7 @@ from pandas import Series, Timestamp from pandas.compat import range, lmap import pandas.core.common as com +from pandas.core import ops import pandas.util.testing as tm @@ -167,26 +168,26 @@ def test_random_state(): def test_maybe_match_name(): - matched = com._maybe_match_name( + matched = ops._maybe_match_name( Series([1], name='x'), Series( [2], name='x')) assert (matched == 'x') - matched = com._maybe_match_name( + matched = ops._maybe_match_name( Series([1], name='x'), Series( [2], name='y')) assert (matched is None) - matched = com._maybe_match_name(Series([1]), Series([2], name='x')) + matched = ops._maybe_match_name(Series([1]), Series([2], name='x')) assert (matched is None) - matched = com._maybe_match_name(Series([1], name='x'), Series([2])) + matched = ops._maybe_match_name(Series([1], name='x'), Series([2])) assert (matched is None) - matched = com._maybe_match_name(Series([1], name='x'), [2]) + matched = ops._maybe_match_name(Series([1], name='x'), [2]) assert (matched == 'x') - matched = com._maybe_match_name([1], Series([2], name='y')) + matched = ops._maybe_match_name([1], Series([2], name='y')) assert (matched == 'y') From d67d8e5a70858e3ecbfe769f89977a0de700c832 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sun, 18 Feb 2018 12:37:56 -0800 Subject: [PATCH 6/9] fix missing copy/paste --- pandas/core/ops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/ops.py b/pandas/core/ops.py index 9208d8fcfcf14..db9da5e10e19d 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -948,7 +948,7 @@ def wrapper(self, other, axis=None): if axis is not None: self._get_axis_number(axis) - res_name = _get_series_op_result_name(self, other) + res_name = get_op_result_name(self, other) if isinstance(other, ABCDataFrame): # pragma: no cover # Defer to DataFrame implementation; fail early From 9bb2f9a83d1a305f6f90bb55e4d73e691c430ea7 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sun, 18 Feb 2018 13:17:16 -0800 Subject: [PATCH 7/9] fix rename inplace --- pandas/core/indexes/datetimelike.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pandas/core/indexes/datetimelike.py b/pandas/core/indexes/datetimelike.py index ecda88057d4ae..03cb821b4af45 100644 --- a/pandas/core/indexes/datetimelike.py +++ b/pandas/core/indexes/datetimelike.py @@ -689,7 +689,7 @@ def __add__(self, other): if result is not NotImplemented: res_name = ops.get_op_result_name(self, other) - result = result.rename(name=res_name, inplace=True) + result.rename(name=res_name, inplace=True) return result cls.__add__ = __add__ @@ -738,7 +738,7 @@ def __sub__(self, other): if result is not NotImplemented: res_name = ops.get_op_result_name(self, other) - result = result.rename(name=res_name, inplace=True) + result.rename(name=res_name, inplace=True) return result cls.__sub__ = __sub__ From 34b25149d3bfd36d614118962add0d30751401c8 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Mon, 19 Feb 2018 19:35:31 -0800 Subject: [PATCH 8/9] replace uses of _maybe_match_name with get_op_result_name --- pandas/core/ops.py | 24 ++++++++++++++++++++++-- pandas/core/series.py | 6 +++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/pandas/core/ops.py b/pandas/core/ops.py index 217b93a9f059d..bdce611f58f06 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -69,12 +69,32 @@ def get_op_result_name(left, right): def _maybe_match_name(a, b): + """ + Try to find a name to attach to the result of an operation between + a and b. If only one of these has a `name` attribute, return that + name. Otherwise return a consensus name if they match of None if + they have different names. + + Parameters + ---------- + a : object + b : object + + Returns + ------- + name : str or None + + See also + -------- + pandas.core.common._consensus_name_attr + """ a_has = hasattr(a, 'name') b_has = hasattr(b, 'name') if a_has and b_has: if a.name == b.name: return a.name else: + # TODO: what if they both have np.nan for their names? return None elif a_has: return a.name @@ -1132,7 +1152,7 @@ def wrapper(self, other): return NotImplemented elif isinstance(other, ABCSeries): - name = _maybe_match_name(self, other) + name = get_op_result_name(self, other) is_other_int_dtype = is_integer_dtype(other.dtype) other = fill_int(other) if is_other_int_dtype else fill_bool(other) @@ -1574,7 +1594,7 @@ def wrapper(self, other): def _sparse_series_op(left, right, op, name): left, right = left.align(right, join='outer', copy=False) new_index = left.index - new_name = _maybe_match_name(left, right) + new_name = get_op_result_name(left, right) from pandas.core.sparse.array import _sparse_array_op result = _sparse_array_op(left.values, right.values, op, name, diff --git a/pandas/core/series.py b/pandas/core/series.py index 4e10dd3f9f5d1..79ffb8be65838 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -1728,7 +1728,7 @@ def _binop(self, other, func, level=None, fill_value=None): with np.errstate(all='ignore'): result = func(this_vals, other_vals) - name = ops._maybe_match_name(self, other) + name = ops.get_op_result_name(self, other) result = self._constructor(result, index=new_index, name=name) result = result.__finalize__(self) if name is None: @@ -1769,7 +1769,7 @@ def combine(self, other, func, fill_value=np.nan): """ if isinstance(other, Series): new_index = self.index.union(other.index) - new_name = ops._maybe_match_name(self, other) + new_name = ops.get_op_result_name(self, other) new_values = np.empty(len(new_index), dtype=self.dtype) for i, idx in enumerate(new_index): lv = self.get(idx, fill_value) @@ -1814,7 +1814,7 @@ def combine_first(self, other): this = self.reindex(new_index, copy=False) other = other.reindex(new_index, copy=False) # TODO: do we need name? - name = ops._maybe_match_name(self, other) # noqa + name = ops.get_op_result_name(self, other) # noqa rs_vals = com._where_compat(isna(this), other._values, this._values) return self._constructor(rs_vals, index=new_index).__finalize__(self) From 249b7ba8c4767c487abf73e2ab10cb6c85616b51 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Tue, 20 Feb 2018 08:09:38 -0800 Subject: [PATCH 9/9] dont use result.rename --- pandas/core/indexes/datetimelike.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pandas/core/indexes/datetimelike.py b/pandas/core/indexes/datetimelike.py index 03cb821b4af45..187f9fcf52dd4 100644 --- a/pandas/core/indexes/datetimelike.py +++ b/pandas/core/indexes/datetimelike.py @@ -689,7 +689,7 @@ def __add__(self, other): if result is not NotImplemented: res_name = ops.get_op_result_name(self, other) - result.rename(name=res_name, inplace=True) + result.name = res_name return result cls.__add__ = __add__ @@ -738,7 +738,7 @@ def __sub__(self, other): if result is not NotImplemented: res_name = ops.get_op_result_name(self, other) - result.rename(name=res_name, inplace=True) + result.name = res_name return result cls.__sub__ = __sub__