Skip to content

REF: separate flex from non-flex DataFrame ops #36843

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

Merged
merged 5 commits into from
Oct 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.2.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ Numeric
- Bug in :class:`Series` flex arithmetic methods where the result when operating with a ``list``, ``tuple`` or ``np.ndarray`` would have an incorrect name (:issue:`36760`)
- Bug in :class:`IntegerArray` multiplication with ``timedelta`` and ``np.timedelta64`` objects (:issue:`36870`)
- Bug in :meth:`DataFrame.diff` with ``datetime64`` dtypes including ``NaT`` values failing to fill ``NaT`` results correctly (:issue:`32441`)
- Bug in :class:`DataFrame` arithmetic ops incorrectly accepting keyword arguments (:issue:`36843`)

Conversion
^^^^^^^^^^
Expand Down
38 changes: 27 additions & 11 deletions pandas/core/ops/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -533,18 +533,13 @@ def _maybe_align_series_as_frame(frame: "DataFrame", series: "Series", axis: int
return type(frame)(rvalues, index=frame.index, columns=frame.columns)


def arith_method_FRAME(cls: Type["DataFrame"], op, special: bool):
# This is the only function where `special` can be either True or False
def flex_arith_method_FRAME(cls: Type["DataFrame"], op, special: bool):
assert not special
op_name = _get_op_name(op, special)
default_axis = None if special else "columns"

na_op = get_array_op(op)

if op_name in _op_descriptions:
# i.e. include "add" but not "__add__"
doc = _make_flex_doc(op_name, "dataframe")
else:
doc = _arith_doc_FRAME % op_name
doc = _make_flex_doc(op_name, "dataframe")

@Appender(doc)
def f(self, other, axis=default_axis, level=None, fill_value=None):
Expand All @@ -561,8 +556,6 @@ def f(self, other, axis=default_axis, level=None, fill_value=None):

axis = self._get_axis_number(axis) if axis is not None else 1

# TODO: why are we passing flex=True instead of flex=not special?
# 15 tests fail if we pass flex=not special instead
self, other = align_method_FRAME(self, other, axis, flex=True, level=level)

if isinstance(other, ABCDataFrame):
Expand All @@ -585,6 +578,29 @@ def f(self, other, axis=default_axis, level=None, fill_value=None):
return f


def arith_method_FRAME(cls: Type["DataFrame"], op, special: bool):
assert special
op_name = _get_op_name(op, special)
doc = _arith_doc_FRAME % op_name

@Appender(doc)
def f(self, other):

if _should_reindex_frame_op(self, other, op, 1, 1, None, None):
return _frame_arith_method_with_reindex(self, other, op)

axis = 1 # only relevant for Series other case

self, other = align_method_FRAME(self, other, axis, flex=True, level=None)

new_data = dispatch_to_series(self, other, op, axis=axis)
return self._construct_result(new_data)

f.__name__ = op_name

return f


def flex_comp_method_FRAME(cls: Type["DataFrame"], op, special: bool):
assert not special # "special" also means "not flex"
op_name = _get_op_name(op, special)
Expand Down Expand Up @@ -616,7 +632,7 @@ def comp_method_FRAME(cls: Type["DataFrame"], op, special: bool):
def f(self, other):
axis = 1 # only relevant for Series other case

self, other = align_method_FRAME(self, other, axis, level=None, flex=False)
self, other = align_method_FRAME(self, other, axis, flex=False, level=None)

# See GH#4537 for discussion of scalar op behavior
new_data = dispatch_to_series(self, other, op, axis=axis)
Expand Down
3 changes: 2 additions & 1 deletion pandas/core/ops/methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def _get_method_wrappers(cls):
from pandas.core.ops import (
arith_method_FRAME,
comp_method_FRAME,
flex_arith_method_FRAME,
flex_comp_method_FRAME,
flex_method_SERIES,
)
Expand All @@ -58,7 +59,7 @@ def _get_method_wrappers(cls):
comp_special = None
bool_special = None
elif issubclass(cls, ABCDataFrame):
arith_flex = arith_method_FRAME
arith_flex = flex_arith_method_FRAME
comp_flex = flex_comp_method_FRAME
arith_special = arith_method_FRAME
comp_special = comp_method_FRAME
Expand Down
7 changes: 7 additions & 0 deletions pandas/tests/frame/test_arithmetic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1484,6 +1484,13 @@ def test_no_warning(self, all_arithmetic_operators):
df = pd.DataFrame({"A": [0.0, 0.0], "B": [0.0, None]})
b = df["B"]
with tm.assert_produces_warning(None):
getattr(df, all_arithmetic_operators)(b)

def test_dunder_methods_binary(self, all_arithmetic_operators):
# GH#??? frame.__foo__ should only accept one argument
df = pd.DataFrame({"A": [0.0, 0.0], "B": [0.0, None]})
b = df["B"]
with pytest.raises(TypeError, match="takes 2 positional arguments"):
getattr(df, all_arithmetic_operators)(b, 0)


Expand Down
13 changes: 0 additions & 13 deletions pandas/tests/series/test_operators.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,25 +276,12 @@ def test_scalar_na_logical_ops_corners_aligns(self):

expected = DataFrame(False, index=range(9), columns=["A"] + list(range(9)))

result = d.__and__(s, axis="columns")
tm.assert_frame_equal(result, expected)

result = d.__and__(s, axis=1)
tm.assert_frame_equal(result, expected)

result = s & d
tm.assert_frame_equal(result, expected)

result = d & s
tm.assert_frame_equal(result, expected)

expected = (s & s).to_frame("A")
result = d.__and__(s, axis="index")
tm.assert_frame_equal(result, expected)

result = d.__and__(s, axis=0)
tm.assert_frame_equal(result, expected)

@pytest.mark.parametrize("op", [operator.and_, operator.or_, operator.xor])
def test_logical_ops_with_index(self, op):
# GH#22092, GH#19792
Expand Down