Skip to content

TST: run frame/series arithmetic tests with+without numexpr #41178

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
Apr 29, 2021
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
38 changes: 31 additions & 7 deletions pandas/tests/frame/test_arithmetic.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
)
import pandas._testing as tm
import pandas.core.common as com
from pandas.core.computation import expressions as expr
from pandas.core.computation.expressions import (
_MIN_ELEMENTS,
NUMEXPR_INSTALLED,
Expand All @@ -27,6 +28,16 @@
)


@pytest.fixture(
autouse=True, scope="module", params=[0, 1000000], ids=["numexpr", "python"]
)
def switch_numexpr_min_elements(request):
_MIN_ELEMENTS = expr._MIN_ELEMENTS
expr._MIN_ELEMENTS = request.param
yield request.param
Copy link
Member Author

@jorisvandenbossche jorisvandenbossche Apr 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copy/pasted this fixture from the tests/arithmetic/. Is there a way to share such an autouse fixture?

A from pandas.tests.arithmetic.conftest import switch_numexpr_min_elements would do the trick, but for some reason we disallow explicit imports from conftest.py files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think this needs to be autouse if you are using this explicity (which you are), so just put it in the conftest in tests/frame

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is autoused by all tests in this file, I didn't add it to each test function signature.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to share such an autouse fixture?

only thing that comes to mind is defining it as non-autouse in a high-level conftest and then

@pytest.fixture(autouse=True)
def switch_numexpr_min_elements(switch_numexpr_min_elements_non_auotuse):
    yield from switch_numexpr_min_elements_non_autouse()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure that can work with a fixture as the base function. Because if a fixture is passed as an argument like this, I think it is already what is yielded from that function that is being passed to the new fixture? (it might be possible, but in any case I don't get it to work)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough; its pretty magical. maybe with yield switch_numexpr_min_elements_non_autouse instead of yield switch_numexpr_min_elements_non_autouse()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I tried that as well (I tried a few variants, and none directly worked), but not fully sure anymore.

expr._MIN_ELEMENTS = _MIN_ELEMENTS


class DummyElement:
def __init__(self, value, dtype):
self.value = value
Expand Down Expand Up @@ -514,7 +525,12 @@ def f(x, y):

@pytest.mark.parametrize("op", ["__add__", "__sub__", "__mul__"])
def test_arith_flex_frame_mixed(
self, op, int_frame, mixed_int_frame, mixed_float_frame
self,
op,
int_frame,
mixed_int_frame,
mixed_float_frame,
switch_numexpr_min_elements,
):
f = getattr(operator, op)

Expand All @@ -528,6 +544,12 @@ def test_arith_flex_frame_mixed(
dtype = {"B": "uint64", "C": None}
elif op in ["__add__", "__mul__"]:
dtype = {"C": None}
if expr.USE_NUMEXPR and switch_numexpr_min_elements == 0:
# when using numexpr, the casting rules are slightly different:
# in the `2 + mixed_int_frame` operation, int32 column becomes
# and int64 column (not preserving dtype in operation with Python
# scalar), and then the int32/int64 combo results in int64 result
dtype["A"] = (2 + mixed_int_frame)["A"].dtype
tm.assert_frame_equal(result, expected)
_check_mixed_int(result, dtype=dtype)

Expand Down Expand Up @@ -892,7 +914,7 @@ def test_frame_with_frame_reindex(self):
],
ids=lambda x: x.__name__,
)
def test_binop_other(self, op, value, dtype):
def test_binop_other(self, op, value, dtype, switch_numexpr_min_elements):

skip = {
(operator.truediv, "bool"),
Expand Down Expand Up @@ -941,11 +963,13 @@ def test_binop_other(self, op, value, dtype):
elif (op, dtype) in skip:

if op in [operator.add, operator.mul]:
# TODO we should assert this or not depending on whether
# numexpr is used or not
# with tm.assert_produces_warning(UserWarning):
# # "evaluating in Python space because ..."
op(s, e.value)
if expr.USE_NUMEXPR and switch_numexpr_min_elements == 0:
# "evaluating in Python space because ..."
warn = UserWarning
else:
warn = None
with tm.assert_produces_warning(warn):
op(s, e.value)

else:
msg = "operator '.*' not implemented for .* dtypes"
Expand Down
11 changes: 11 additions & 0 deletions pandas/tests/series/test_arithmetic.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@
nanops,
ops,
)
from pandas.core.computation import expressions as expr


@pytest.fixture(
autouse=True, scope="module", params=[0, 1000000], ids=["numexpr", "python"]
)
def switch_numexpr_min_elements(request):
_MIN_ELEMENTS = expr._MIN_ELEMENTS
expr._MIN_ELEMENTS = request.param
yield request.param
expr._MIN_ELEMENTS = _MIN_ELEMENTS


def _permute(obj):
Expand Down