From cba3b471d088fca5682f775de3155adfa6e36cb0 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Thu, 13 Sep 2018 10:25:05 -0700 Subject: [PATCH 1/3] BUG: fix DataFrame+DataFrame op with timedelta64 dtype --- pandas/core/frame.py | 9 +-------- pandas/core/ops.py | 5 +++-- pandas/tests/frame/test_arithmetic.py | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 251bc6587872d..6d707bbffe30e 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -4827,14 +4827,7 @@ def _arith_op(left, right): left, right = ops.fill_binop(left, right, fill_value) return func(left, right) - if this._is_mixed_type or other._is_mixed_type: - # iterate over columns - return ops.dispatch_to_series(this, other, _arith_op) - else: - result = _arith_op(this.values, other.values) - return self._constructor(result, - index=new_index, columns=new_columns, - copy=False) + return ops.dispatch_to_series(this, other, _arith_op) def _combine_match_index(self, other, func, level=None): assert isinstance(other, Series) diff --git a/pandas/core/ops.py b/pandas/core/ops.py index ca9c2528f0aef..2d6b365faac35 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -1794,8 +1794,9 @@ def f(self, other, axis=default_axis, level=None, fill_value=None): other = _align_method_FRAME(self, other, axis) - if isinstance(other, ABCDataFrame): # Another DataFrame - return self._combine_frame(other, na_op, fill_value, level) + if isinstance(other, ABCDataFrame): + # Another DataFrame + return self._combine_frame(other, op, fill_value, level) elif isinstance(other, ABCSeries): return _combine_series_frame(self, other, na_op, fill_value=fill_value, axis=axis, diff --git a/pandas/tests/frame/test_arithmetic.py b/pandas/tests/frame/test_arithmetic.py index a6f4e0e38ec5d..290b7add9fc1b 100644 --- a/pandas/tests/frame/test_arithmetic.py +++ b/pandas/tests/frame/test_arithmetic.py @@ -299,3 +299,18 @@ def test_df_bool_mul_int(self): result = 1 * df kinds = result.dtypes.apply(lambda x: x.kind) assert (kinds == 'i').all() + + def test_td64_df_add_int_frame(self): + # Check that we don't dispatch to numpy implementation, which treats + # int64 as m8[ns] + tdi = pd.timedelta_range('1', periods=3) + df = tdi.to_frame() + other = pd.DataFrame([1, 2, 3], index=tdi) # indexed like `df` + with pytest.raises(TypeError): + df + other + with pytest.raises(TypeError): + other + df + with pytest.raises(TypeError): + df - other + with pytest.raises(TypeError): + other - df From d4e4353444eb06b29683aed06bbe182aa88c54ef Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Fri, 28 Sep 2018 15:26:18 -0700 Subject: [PATCH 2/3] Implement should_series_dispatch --- pandas/core/frame.py | 9 ++++++- pandas/core/ops.py | 39 ++++++++++++++++++++++++++- pandas/tests/frame/test_arithmetic.py | 4 +-- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/pandas/core/frame.py b/pandas/core/frame.py index 8dabf1721d74e..b00a9e1013b02 100644 --- a/pandas/core/frame.py +++ b/pandas/core/frame.py @@ -4886,7 +4886,14 @@ def _arith_op(left, right): left, right = ops.fill_binop(left, right, fill_value) return func(left, right) - return ops.dispatch_to_series(this, other, _arith_op) + if ops.should_series_dispatch(this, other, func): + # iterate over columns + return ops.dispatch_to_series(this, other, _arith_op) + else: + result = _arith_op(this.values, other.values) + return self._constructor(result, + index=new_index, columns=new_columns, + copy=False) def _combine_match_index(self, other, func, level=None): assert isinstance(other, Series) diff --git a/pandas/core/ops.py b/pandas/core/ops.py index b532e06fcffdf..3e02dd85b6b1e 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -900,6 +900,42 @@ def invalid_comparison(left, right, op): return res_values +# ----------------------------------------------------------------------------- +# Dispatch logic + +def should_series_dispatch(left, right, op): + """ + Identify cases where a DataFrame operation should dispatch to its + Series counterpart. + + Parameters + ---------- + left : DataFrame + right : DataFrame + op : binary operator + + Returns + ------- + override : bool + """ + if left._is_mixed_type or right._is_mixed_type: + return True + + if not len(left.columns) or not len(right.columns): + # ensure obj.dtypes[0] exists for each obj + return False + + ldtype = left.dtypes.iloc[0] + rdtype = right.dtypes.iloc[0] + + if ((is_timedelta64_dtype(ldtype) and is_integer_dtype(rdtype)) or + (is_timedelta64_dtype(rdtype) and is_integer_dtype(ldtype))): + # numpy integer dtypes as timedelta64 dtypes in this scenario + return True + + return False + + # ----------------------------------------------------------------------------- # Functions that add arithmetic methods to objects, given arithmetic factory # methods @@ -1804,7 +1840,8 @@ def f(self, other, axis=default_axis, level=None, fill_value=None): if isinstance(other, ABCDataFrame): # Another DataFrame - return self._combine_frame(other, op, fill_value, level) + pass_op = op if should_series_dispatch(self, other, op) else na_op + return self._combine_frame(other, pass_op, fill_value, level) elif isinstance(other, ABCSeries): return _combine_series_frame(self, other, na_op, fill_value=fill_value, axis=axis, diff --git a/pandas/tests/frame/test_arithmetic.py b/pandas/tests/frame/test_arithmetic.py index b7255405f7011..2eb11c3a2e2f7 100644 --- a/pandas/tests/frame/test_arithmetic.py +++ b/pandas/tests/frame/test_arithmetic.py @@ -268,8 +268,8 @@ def test_df_bool_mul_int(self): assert (kinds == 'i').all() def test_td64_df_add_int_frame(self): - # Check that we don't dispatch to numpy implementation, which treats - # int64 as m8[ns] + # GH#22696 Check that we don't dispatch to numpy implementation, + # which treats int64 as m8[ns] tdi = pd.timedelta_range('1', periods=3) df = tdi.to_frame() other = pd.DataFrame([1, 2, 3], index=tdi) # indexed like `df` From d6f7fbf261e7528e32721b6707c303bd8d71ef7d Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Mon, 1 Oct 2018 18:38:25 -0700 Subject: [PATCH 3/3] Whatsnew note --- doc/source/whatsnew/v0.24.0.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index 851c1a3fbd6e9..a240f1fd85dd0 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -666,7 +666,7 @@ Timedelta - Bug in :class:`Index` with numeric dtype when multiplying or dividing an array with dtype ``timedelta64`` (:issue:`22390`) - Bug in :class:`TimedeltaIndex` incorrectly allowing indexing with ``Timestamp`` object (:issue:`20464`) - Fixed bug where subtracting :class:`Timedelta` from an object-dtyped array would raise ``TypeError`` (:issue:`21980`) -- +- Fixed bug in adding a :class:`DataFrame` with all-`timedelta64[ns]` dtypes to a :class:`DataFrame` with all-integer dtypes returning incorrect results instead of raising ``TypeError`` (:issue:`22696`) - Timezones