From 23fcf74da49c120f8df0b7945b32fc2e4eb26f23 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Wed, 24 Oct 2018 09:30:31 -0700 Subject: [PATCH 1/6] remove unused errors kwarg --- pandas/core/frame.py | 2 +- pandas/core/ops.py | 3 +-- pandas/core/sparse/frame.py | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 6dd9174028f18..d3dcae48dc6a0 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -4978,7 +4978,7 @@ def _combine_match_columns(self, other, func, level=None): assert left.columns.equals(right.index) return ops.dispatch_to_series(left, right, func, axis="columns") - def _combine_const(self, other, func, errors='raise'): + def _combine_const(self, other, func): assert lib.is_scalar(other) or np.ndim(other) == 0 return ops.dispatch_to_series(self, other, func) diff --git a/pandas/core/ops.py b/pandas/core/ops.py index f29b4410fbf54..fc862c90402e1 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -1940,8 +1940,7 @@ def f(self, other): # straight boolean comparisons we want to allow all columns # (regardless of dtype to pass thru) See #4537 for discussion. - res = self._combine_const(other, func, - errors='ignore') + res = self._combine_const(other, func) return res.fillna(True).astype(bool) f.__name__ = op_name diff --git a/pandas/core/sparse/frame.py b/pandas/core/sparse/frame.py index e46df2b2bde70..56a5f4f49243f 100644 --- a/pandas/core/sparse/frame.py +++ b/pandas/core/sparse/frame.py @@ -642,7 +642,7 @@ def _combine_match_columns(self, other, func, level=None): new_data, index=self.index, columns=union, default_fill_value=self.default_fill_value).__finalize__(self) - def _combine_const(self, other, func, errors='raise'): + def _combine_const(self, other, func): return self._apply_columns(lambda x: func(x, other)) def _reindex_index(self, index, method, copy, level, fill_value=np.nan, From 7755b2431b8a39d64496045d1cb5d4b2d74e3777 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Wed, 24 Oct 2018 09:41:27 -0700 Subject: [PATCH 2/6] Fix operations with Tick, timedelta64, RangeIndex --- pandas/core/indexes/range.py | 8 ++++++-- pandas/core/ops.py | 11 ++++++++++- pandas/tests/arithmetic/conftest.py | 6 ++++-- pandas/tests/arithmetic/test_numeric.py | 8 -------- pandas/tests/arithmetic/test_timedelta64.py | 6 ++---- pandas/tests/indexes/test_range.py | 19 +++++++++++++++++++ pandas/tests/internals/test_internals.py | 1 - 7 files changed, 41 insertions(+), 18 deletions(-) diff --git a/pandas/core/indexes/range.py b/pandas/core/indexes/range.py index 2b2f9ca51ce12..8f2399cfa10a2 100644 --- a/pandas/core/indexes/range.py +++ b/pandas/core/indexes/range.py @@ -16,7 +16,8 @@ from pandas.core.dtypes.common import ( is_int64_dtype, is_integer, is_scalar, is_timedelta64_dtype ) -from pandas.core.dtypes.generic import ABCSeries, ABCTimedeltaIndex +from pandas.core.dtypes.generic import ( + ABCDataFrame, ABCSeries, ABCTimedeltaIndex) from pandas.core.indexes.base import Index, _index_shared_docs from pandas.core.indexes.numeric import Int64Index from pandas.util._decorators import Appender, cache_readonly @@ -557,6 +558,9 @@ def __getitem__(self, key): return super_getitem(key) def __floordiv__(self, other): + if isinstance(other, (ABCSeries, ABCDataFrame)): + return NotImplemented + if is_integer(other) and other != 0: if (len(self) == 0 or self._start % other == 0 and @@ -588,7 +592,7 @@ def _make_evaluate_binop(op, step=False): """ def _evaluate_numeric_binop(self, other): - if isinstance(other, ABCSeries): + if isinstance(other, (ABCSeries, ABCDataFrame)): return NotImplemented elif isinstance(other, ABCTimedeltaIndex): # Defer to TimedeltaIndex implementation diff --git a/pandas/core/ops.py b/pandas/core/ops.py index fc862c90402e1..07532f7ec18be 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -45,6 +45,8 @@ ABCIndex, ABCIndexClass, ABCSparseSeries, ABCSparseArray) +from pandas.tseries.offsets import Tick + # ----------------------------------------------------------------------------- # Ops Wrapping Utilities @@ -125,11 +127,18 @@ def maybe_upcast_for_op(obj): Be careful to call this *after* determining the `name` attribute to be attached to the result of the arithmetic operation. """ - if type(obj) is datetime.timedelta: + if type(obj) is datetime.timedelta or isinstance(obj, Tick): # GH#22390 cast up to Timedelta to rely on Timedelta # implementation; otherwise operation against numeric-dtype # raises TypeError return pd.Timedelta(obj) + elif isinstance(obj, np.timedelta64) and not isna(obj): + # In particular non-nanosecond timedelta64 needs to be cast to + # nanoseconds, or else we get undesired behavior like + # np.timedelta64(3, 'D') / 2 == np.timedelta64(1, 'D') + # The isna check is to avoid casting timedelta64("NaT"), which would + # return NaT and incorrectly be treated as a datetime-NaT. + return pd.Timedelta(obj) elif isinstance(obj, np.ndarray) and is_timedelta64_dtype(obj): # GH#22390 Unfortunately we need to special-case right-hand # timedelta64 dtypes because numpy casts integer dtypes to diff --git a/pandas/tests/arithmetic/conftest.py b/pandas/tests/arithmetic/conftest.py index b800b66e8edea..cbe26a06d34c6 100644 --- a/pandas/tests/arithmetic/conftest.py +++ b/pandas/tests/arithmetic/conftest.py @@ -70,7 +70,8 @@ def scalar_td(request): pd.Timedelta(days=3).to_pytimedelta(), pd.Timedelta('72:00:00'), np.timedelta64(3, 'D'), - np.timedelta64(72, 'h')]) + np.timedelta64(72, 'h')], + ids=lambda x: type(x).__name__) def three_days(request): """ Several timedelta-like and DateOffset objects that each represent @@ -84,7 +85,8 @@ def three_days(request): pd.Timedelta(hours=2).to_pytimedelta(), pd.Timedelta(seconds=2 * 3600), np.timedelta64(2, 'h'), - np.timedelta64(120, 'm')]) + np.timedelta64(120, 'm')], + ids=lambda x: type(x).__name__) def two_hours(request): """ Several timedelta-like and DateOffset objects that each represent diff --git a/pandas/tests/arithmetic/test_numeric.py b/pandas/tests/arithmetic/test_numeric.py index 25845dd8b3151..b8b88630a4344 100644 --- a/pandas/tests/arithmetic/test_numeric.py +++ b/pandas/tests/arithmetic/test_numeric.py @@ -150,14 +150,6 @@ def test_numeric_arr_mul_tdscalar(self, scalar_td, numeric_idx, box): def test_numeric_arr_rdiv_tdscalar(self, three_days, numeric_idx, box): index = numeric_idx[1:3] - broken = (isinstance(three_days, np.timedelta64) and - three_days.dtype != 'm8[ns]') - broken = broken or isinstance(three_days, pd.offsets.Tick) - if box is not pd.Index and broken: - # np.timedelta64(3, 'D') / 2 == np.timedelta64(1, 'D') - raise pytest.xfail("timedelta64 not converted to nanos; " - "Tick division not implemented") - expected = TimedeltaIndex(['3 Days', '36 Hours']) index = tm.box_expected(index, box) diff --git a/pandas/tests/arithmetic/test_timedelta64.py b/pandas/tests/arithmetic/test_timedelta64.py index 56bef2fee2b41..e646aad5ba48e 100644 --- a/pandas/tests/arithmetic/test_timedelta64.py +++ b/pandas/tests/arithmetic/test_timedelta64.py @@ -1033,10 +1033,8 @@ def test_tdi_mul_float_series(self, box_df_fail): pd.Float64Index(range(1, 11)), pd.RangeIndex(1, 11) ], ids=lambda x: type(x).__name__) - def test_tdi_rmul_arraylike(self, other, box_df_fail): - # RangeIndex fails to return NotImplemented, for others - # DataFrame tries to broadcast incorrectly - box = box_df_fail + def test_tdi_rmul_arraylike(self, other, box_df_broadcast_failure): + box = box_df_broadcast_failure tdi = TimedeltaIndex(['1 Day'] * 10) expected = timedelta_range('1 days', '10 days') diff --git a/pandas/tests/indexes/test_range.py b/pandas/tests/indexes/test_range.py index 0e47fcd52f625..fd2df39b1e7e7 100644 --- a/pandas/tests/indexes/test_range.py +++ b/pandas/tests/indexes/test_range.py @@ -189,6 +189,25 @@ def test_constructor_name(self): assert copy.name == 'copy' assert new.name == 'new' + # TODO: mod, divmod? + @pytest.mark.parametrize('op', [operator.add, operator.sub, + operator.mul, operator.floordiv, + operator.truediv, operator.pow]) + def test_arithmetic_with_frame_or_series(self, op): + # check that we return NotImplemented when operating with Series + # or DataFrame + index = pd.RangeIndex(5) + other = pd.Series(np.random.randn(5)) + + expected = op(pd.Series(index), other) + result = op(index, other) + tm.assert_series_equal(result, expected) + + other = pd.DataFrame(np.random.randn(2, 5)) + expected = op(pd.DataFrame([index, index]), other) + result = op(index, other) + tm.assert_frame_equal(result, expected) + def test_numeric_compat2(self): # validate that we are handling the RangeIndex overrides to numeric ops # and returning RangeIndex where possible diff --git a/pandas/tests/internals/test_internals.py b/pandas/tests/internals/test_internals.py index cdf35ea96588a..b327b158adc24 100644 --- a/pandas/tests/internals/test_internals.py +++ b/pandas/tests/internals/test_internals.py @@ -1243,7 +1243,6 @@ def test_binop_other(self, op, value, dtype): (operator.mul, ' Date: Wed, 24 Oct 2018 10:16:39 -0700 Subject: [PATCH 3/6] Fix and test timedelta64(nat) and timedetal-with-numeric-index ops --- pandas/core/indexes/base.py | 7 +++++++ pandas/core/ops.py | 9 +++++---- pandas/tests/arithmetic/test_numeric.py | 20 ++++++++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/pandas/core/indexes/base.py b/pandas/core/indexes/base.py index e5760f0141efb..8c91a9d570bab 100644 --- a/pandas/core/indexes/base.py +++ b/pandas/core/indexes/base.py @@ -4698,6 +4698,13 @@ def dropna(self, how='any'): def _evaluate_with_timedelta_like(self, other, op): # Timedelta knows how to operate with np.array, so dispatch to that # operation and then wrap the results + if self._is_numeric_dtype and op.__name__ in ['add', 'sub', + 'radd', 'rsub']: + raise TypeError("Operation {opname} between {cls} and {other} " + "is invalid".format(opname=op.__name__, + cls=type(self).__name__, + other=type(other).__name__)) + other = Timedelta(other) values = self.values diff --git a/pandas/core/ops.py b/pandas/core/ops.py index 07532f7ec18be..79faa5def3a07 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -1316,11 +1316,12 @@ def wrapper(left, right): index=left.index, name=res_name, dtype=result.dtype) - elif is_timedelta64_dtype(right) and not is_scalar(right): - # i.e. exclude np.timedelta64 object + elif is_timedelta64_dtype(right): + # We should only get here with non-scalar or timedelta64('NaT') + # values for right # Note: we cannot use dispatch_to_index_op because - # that may incorrectly raise TypeError when we - # should get NullFrequencyError + # that may incorrectly raise TypeError when we + # should get NullFrequencyError result = op(pd.Index(left), right) return construct_result(left, result, index=left.index, name=res_name, diff --git a/pandas/tests/arithmetic/test_numeric.py b/pandas/tests/arithmetic/test_numeric.py index b8b88630a4344..043f0a85b5cbf 100644 --- a/pandas/tests/arithmetic/test_numeric.py +++ b/pandas/tests/arithmetic/test_numeric.py @@ -161,6 +161,26 @@ def test_numeric_arr_rdiv_tdscalar(self, three_days, numeric_idx, box): with pytest.raises(TypeError): index / three_days + @pytest.mark.parametrize('other', [ + pd.Timedelta(hours=31), + pd.Timedelta(hours=31).to_pytimedelta(), + pd.Timedelta(hours=31).to_timedelta64(), + pd.Timedelta(hours=31).to_timedelta64().astype('m8[h]'), + np.timedelta64('NaT'), + np.timedelta64('NaT', 'D'), + pd.offsets.Minute(3), + pd.offsets.Second(0)]) + def test_add_sub_timedeltalike_invalid(self, numeric_idx, other, box): + left = tm.box_expected(numeric_idx, box) + with pytest.raises(TypeError): + left + other + with pytest.raises(TypeError): + other + left + with pytest.raises(TypeError): + left - other + with pytest.raises(TypeError): + other - left + # ------------------------------------------------------------------ # Arithmetic From 92f1eee71a294c77fb4e6f91c943166dffb051c4 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Wed, 24 Oct 2018 21:21:17 -0700 Subject: [PATCH 4/6] apply isort --- pandas/core/indexes/range.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pandas/core/indexes/range.py b/pandas/core/indexes/range.py index 8f2399cfa10a2..2b111a2915b8f 100644 --- a/pandas/core/indexes/range.py +++ b/pandas/core/indexes/range.py @@ -17,7 +17,8 @@ is_int64_dtype, is_integer, is_scalar, is_timedelta64_dtype ) from pandas.core.dtypes.generic import ( - ABCDataFrame, ABCSeries, ABCTimedeltaIndex) + ABCDataFrame, ABCSeries, ABCTimedeltaIndex +) from pandas.core.indexes.base import Index, _index_shared_docs from pandas.core.indexes.numeric import Int64Index from pandas.util._decorators import Appender, cache_readonly From 8217977711e73522ab648fe6d837eb97d9c34753 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Thu, 25 Oct 2018 17:41:20 -0700 Subject: [PATCH 5/6] combine type check --- 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 79faa5def3a07..237540ea2366c 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -127,7 +127,7 @@ def maybe_upcast_for_op(obj): Be careful to call this *after* determining the `name` attribute to be attached to the result of the arithmetic operation. """ - if type(obj) is datetime.timedelta or isinstance(obj, Tick): + if isinstance(obj, (datetime.timedelta, Tick)): # GH#22390 cast up to Timedelta to rely on Timedelta # implementation; otherwise operation against numeric-dtype # raises TypeError From 8d96c70b6ff2ce05ef88d4fd205483fd2159322f Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 26 Oct 2018 10:00:26 -0700 Subject: [PATCH 6/6] un-fix Tick case --- pandas/core/ops.py | 4 +--- pandas/tests/arithmetic/test_numeric.py | 4 ++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pandas/core/ops.py b/pandas/core/ops.py index 237540ea2366c..1e5baea52abee 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -45,8 +45,6 @@ ABCIndex, ABCIndexClass, ABCSparseSeries, ABCSparseArray) -from pandas.tseries.offsets import Tick - # ----------------------------------------------------------------------------- # Ops Wrapping Utilities @@ -127,7 +125,7 @@ def maybe_upcast_for_op(obj): Be careful to call this *after* determining the `name` attribute to be attached to the result of the arithmetic operation. """ - if isinstance(obj, (datetime.timedelta, Tick)): + if type(obj) is datetime.timedelta: # GH#22390 cast up to Timedelta to rely on Timedelta # implementation; otherwise operation against numeric-dtype # raises TypeError diff --git a/pandas/tests/arithmetic/test_numeric.py b/pandas/tests/arithmetic/test_numeric.py index 043f0a85b5cbf..9163f2e1a3d1c 100644 --- a/pandas/tests/arithmetic/test_numeric.py +++ b/pandas/tests/arithmetic/test_numeric.py @@ -148,6 +148,10 @@ def test_numeric_arr_mul_tdscalar(self, scalar_td, numeric_idx, box): tm.assert_equal(commute, expected) def test_numeric_arr_rdiv_tdscalar(self, three_days, numeric_idx, box): + + if box is not pd.Index and isinstance(three_days, pd.offsets.Tick): + raise pytest.xfail("Tick division not implemented") + index = numeric_idx[1:3] expected = TimedeltaIndex(['3 Days', '36 Hours'])