From 3629e9ec7960aee9d75c1fcaaf18c7be93be65bf Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 23 Jan 2020 16:14:44 -0800 Subject: [PATCH 1/4] REF: only check fill_value in the one place it is needed --- pandas/core/ops/__init__.py | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/pandas/core/ops/__init__.py b/pandas/core/ops/__init__.py index 1355060efd097..a9d149323b465 100644 --- a/pandas/core/ops/__init__.py +++ b/pandas/core/ops/__init__.py @@ -584,17 +584,16 @@ def flex_wrapper(self, other, level=None, fill_value=None, axis=0): # DataFrame -def _combine_series_frame(self, other, func, fill_value=None, axis=None, level=None): +def _combine_series_frame(self, other, func, axis=None, level=None): """ Apply binary operator `func` to self, other using alignment and fill - conventions determined by the fill_value, axis, and level kwargs. + conventions determined by the axis and level kwargs. Parameters ---------- self : DataFrame other : Series func : binary operator - fill_value : object, default None axis : {0, 1, 'columns', 'index', None}, default None level : int or None, default None @@ -602,9 +601,6 @@ def _combine_series_frame(self, other, func, fill_value=None, axis=None, level=N ------- result : DataFrame """ - if fill_value is not None: - raise NotImplementedError(f"fill_value {fill_value} not supported.") - if axis is None: # default axis is columns axis = 1 @@ -707,9 +703,11 @@ def f(self, other, axis=default_axis, level=None, fill_value=None): # so do not want the masked op. pass_op = op if axis in [0, "columns", None] else na_op pass_op = pass_op if not is_logical else op - return _combine_series_frame( - self, other, pass_op, fill_value=fill_value, axis=axis, level=level - ) + + if fill_value is not None: + raise NotImplementedError(f"fill_value {fill_value} not supported.") + + return _combine_series_frame(self, other, pass_op, axis=axis, level=level) else: # in this case we always have `np.ndim(other) == 0` if fill_value is not None: @@ -745,9 +743,7 @@ def f(self, other, axis=default_axis, level=None): return self._construct_result(new_data) elif isinstance(other, ABCSeries): - return _combine_series_frame( - self, other, op, fill_value=None, axis=axis, level=level - ) + return _combine_series_frame(self, other, op, axis=axis, level=level) else: # in this case we always have `np.ndim(other) == 0` new_data = dispatch_to_series(self, other, op) @@ -777,9 +773,7 @@ def f(self, other): return self._construct_result(new_data) elif isinstance(other, ABCSeries): - return _combine_series_frame( - self, other, op, fill_value=None, axis=None, level=None - ) + return _combine_series_frame(self, other, op, axis=None, level=None) else: # straight boolean comparisons we want to allow all columns From f42ea4f3481cc90e40be8ee6586aa7f0053ae9d7 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 23 Jan 2020 16:18:53 -0800 Subject: [PATCH 2/4] REF: move align calls to before combine_series_frame --- pandas/core/ops/__init__.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/pandas/core/ops/__init__.py b/pandas/core/ops/__init__.py index a9d149323b465..52729f81d9256 100644 --- a/pandas/core/ops/__init__.py +++ b/pandas/core/ops/__init__.py @@ -584,29 +584,23 @@ def flex_wrapper(self, other, level=None, fill_value=None, axis=0): # DataFrame -def _combine_series_frame(self, other, func, axis=None, level=None): +def _combine_series_frame(left, right, func, axis: int): """ Apply binary operator `func` to self, other using alignment and fill - conventions determined by the axis and level kwargs. + conventions determined by the axis argument. Parameters ---------- - self : DataFrame - other : Series + left : DataFrame + right : Series func : binary operator - axis : {0, 1, 'columns', 'index', None}, default None - level : int or None, default None + axis : {0, 1} Returns ------- result : DataFrame """ - if axis is None: - # default axis is columns - axis = 1 - - axis = self._get_axis_number(axis) - left, right = self.align(other, join="outer", axis=axis, level=level, copy=False) + # We assume that self.align(other, ...) has already been called if axis == 0: new_data = left._combine_match_index(right, func) else: @@ -707,7 +701,9 @@ def f(self, other, axis=default_axis, level=None, fill_value=None): if fill_value is not None: raise NotImplementedError(f"fill_value {fill_value} not supported.") - return _combine_series_frame(self, other, pass_op, axis=axis, level=level) + axis = self._get_axis_number(axis) if axis is not None else 1 + self, other = self.align(other, join="outer", axis=axis, level=level, copy=False) + return _combine_series_frame(self, other, pass_op, axis=axis) else: # in this case we always have `np.ndim(other) == 0` if fill_value is not None: @@ -743,7 +739,9 @@ def f(self, other, axis=default_axis, level=None): return self._construct_result(new_data) elif isinstance(other, ABCSeries): - return _combine_series_frame(self, other, op, axis=axis, level=level) + axis = self._get_axis_number(axis) if axis is not None else 1 + self, other = self.align(other, join="outer", axis=axis, level=level, copy=False) + return _combine_series_frame(self, other, op, axis=axis) else: # in this case we always have `np.ndim(other) == 0` new_data = dispatch_to_series(self, other, op) @@ -773,7 +771,9 @@ def f(self, other): return self._construct_result(new_data) elif isinstance(other, ABCSeries): - return _combine_series_frame(self, other, op, axis=None, level=None) + # axis=1 is default for DataFrame-with-Series op + self, other = self.align(other, join="outer", axis=1, level=None, copy=False) + return _combine_series_frame(self, other, op, axis=1) else: # straight boolean comparisons we want to allow all columns From 791d8b18c1ff53f136a9699d7f490a6a8483d700 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 23 Jan 2020 17:52:13 -0800 Subject: [PATCH 3/4] avoid call to combine_series_frame --- pandas/core/ops/__init__.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pandas/core/ops/__init__.py b/pandas/core/ops/__init__.py index 52729f81d9256..7d9f3e39b92ab 100644 --- a/pandas/core/ops/__init__.py +++ b/pandas/core/ops/__init__.py @@ -702,7 +702,9 @@ def f(self, other, axis=default_axis, level=None, fill_value=None): raise NotImplementedError(f"fill_value {fill_value} not supported.") axis = self._get_axis_number(axis) if axis is not None else 1 - self, other = self.align(other, join="outer", axis=axis, level=level, copy=False) + self, other = self.align( + other, join="outer", axis=axis, level=level, copy=False + ) return _combine_series_frame(self, other, pass_op, axis=axis) else: # in this case we always have `np.ndim(other) == 0` @@ -740,7 +742,9 @@ def f(self, other, axis=default_axis, level=None): elif isinstance(other, ABCSeries): axis = self._get_axis_number(axis) if axis is not None else 1 - self, other = self.align(other, join="outer", axis=axis, level=level, copy=False) + self, other = self.align( + other, join="outer", axis=axis, level=level, copy=False + ) return _combine_series_frame(self, other, op, axis=axis) else: # in this case we always have `np.ndim(other) == 0` @@ -772,8 +776,12 @@ def f(self, other): elif isinstance(other, ABCSeries): # axis=1 is default for DataFrame-with-Series op - self, other = self.align(other, join="outer", axis=1, level=None, copy=False) - return _combine_series_frame(self, other, op, axis=1) + self, other = self.align( + other, join="outer", axis=1, level=None, copy=False + ) + new_data = dispatch_to_series(self, other, op, axis="columns") + return self._construct_result(new_data) + else: # straight boolean comparisons we want to allow all columns From 0d4520b3cecdeaebdbf8c096bc92761fd0f7e521 Mon Sep 17 00:00:00 2001 From: jbrockmendel Date: Thu, 23 Jan 2020 17:53:45 -0800 Subject: [PATCH 4/4] de-duplicate construct_result calls --- pandas/core/ops/__init__.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pandas/core/ops/__init__.py b/pandas/core/ops/__init__.py index 7d9f3e39b92ab..9ed233cad65ce 100644 --- a/pandas/core/ops/__init__.py +++ b/pandas/core/ops/__init__.py @@ -772,7 +772,6 @@ def f(self, other): "Can only compare identically-labeled DataFrame objects" ) new_data = dispatch_to_series(self, other, op, str_rep) - return self._construct_result(new_data) elif isinstance(other, ABCSeries): # axis=1 is default for DataFrame-with-Series op @@ -780,14 +779,14 @@ def f(self, other): other, join="outer", axis=1, level=None, copy=False ) new_data = dispatch_to_series(self, other, op, axis="columns") - return self._construct_result(new_data) else: # straight boolean comparisons we want to allow all columns # (regardless of dtype to pass thru) See #4537 for discussion. new_data = dispatch_to_series(self, other, op) - return self._construct_result(new_data) + + return self._construct_result(new_data) f.__name__ = op_name