Skip to content

TST: Added regression test case #31536

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.

Sign up for GitHub

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

Closed
ShaharNaveh wants to merge 10 commits into from
Closed

TST: Added regression test case #31536

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ce26841
TST: Added regression test case
Feb 1, 2020
fe01b37
Skipping test if running on 32-bit platform
Feb 1, 2020
2680f0b
Merge remote-tracking branch 'upstream/master' into TST-reg-boolean
Feb 7, 2020
a699369
Removed "skipif" on 32-bit platforms
Feb 7, 2020
69fb424
Replaced "pytest.skip" with "pytest.xfail"
Feb 7, 2020
b38f9e6
Lint issues
Feb 7, 2020
1f3d34f
Merge remote-tracking branch 'upstream/master' into TST-reg-boolean
Feb 22, 2020
da31af0
Added more params to paramaterize
Feb 22, 2020
f97720d
Make use of the normal pattern
Feb 22, 2020
3f7875c
Merge remote-tracking branch 'upstream/master' into TST-reg-boolean
Apr 6, 2020
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
32 changes: 32 additions & 0 deletions pandas/tests/arithmetic/test_numeric.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1305,3 +1305,35 @@ def test_dataframe_div_silenced():
)
with tm.assert_produces_warning(None):
pdf1.div(pdf2, fill_value=0)


class TestNumericArraylikeArithmeticWithBool:
@pytest.mark.parametrize("num", [complex(1), np.int64(1), 1, 1.0])
def test_array_like_bool_and_num_op_coerce(
self, num, all_arithmetic_functions, box_with_array
):
# https://github.com/pandas-dev/pandas/issues/18549
op = all_arithmetic_functions

if op.__name__ in [
"floordiv",
"mod",
"mul",
"pow",
"rfloordiv",
"rpow",
"rmod",
"rmul",
"rtruediv",
"truediv",
]:
pytest.xfail("Arithmetic operation is not supported")
Copy link
Member

@jbrockmendel jbrockmendel Feb 12, 2020

Choose a reason for hiding this comment

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

is this not supported as in "not meaningful and so never will be supported" or as in "we havent gotten around to it yet so the behavior is wrong"? only the latter should be xfail

Copy link
Contributor Author

@ShaharNaveh ShaharNaveh Feb 13, 2020

Choose a reason for hiding this comment

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

I am not sure, maybe @jorisvandenbossche knows?

Copy link
Member

@jorisvandenbossche jorisvandenbossche Feb 13, 2020

Choose a reason for hiding this comment

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

It's hard to see what the test is actually testing, but I would think something like this?

In [5]: pd.Series([True]) * 3 
Out[5]: 
0    3
dtype: int64

but that doesn't fail, though.

But I agree with @jbrockmendel: if there are cases that it raises an error purposefully, we should test that and not xfail.

Copy link
Contributor Author

@ShaharNaveh ShaharNaveh Feb 14, 2020

Choose a reason for hiding this comment

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

I am not able to test this without having if/else statement in the code, which is not really a pattern we want in our tests (I think), Any thought on how to write this test case?

Copy link
Member

@jorisvandenbossche jorisvandenbossche Feb 20, 2020

Choose a reason for hiding this comment

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

The if/else might be fine, but in this if block, you can do a pytest.raises instead of xfail?

Copy link
Contributor Author

@ShaharNaveh ShaharNaveh Feb 22, 2020

Choose a reason for hiding this comment

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

Trying to test the actual errors, will result in having a test case that looks somewhat like this:

   @pytest.mark.parametrize(
        "op",
        [
            operator.mul,
            ops.rmul,
            operator.floordiv,
            ops.rfloordiv,
            operator.truediv,
            ops.rtruediv,
            operator.pow,
            ops.rpow,
            operator.mod,
            ops.rmod,
        ],
    )
    @pytest.mark.parametrize("num", [np.int64(1), 1, 1.0])
    def test_array_like_bool_and_num_op_coerce_raises(self, op, num, box_with_array):
        # https://github.com/pandas-dev/pandas/issues/18549
        bool_box = [True]

        if op.__name__ in {"foo", "bar", "baz"}:
            msg = "|".join(
                [
                    "can't mod complex numbers",
                    "can't take floor of complex number",
                    r"unsupported operand type\(s\) for %: 'float' and 'Index'",
                    "cannot perform __foo__ with this index type: Index",
                ]
            )

            with pytest.raises(TypeError, match=msg):
                expected = [op(num, num)]
            return
        else:
            expected = [op(num, num)]

        bool_box = tm.box_expected(bool_box, box_with_array)
        expected = tm.box_expected(expected, box_with_array)

        if op.__name__ in {"foo", "bar", "baz"}:
            msg = "|".join(["cannot perform __foo__ with this index type: Index"])
            with pytest.raises(TypeError, match=msg):
                tm.assert_equal(expected, op(bool_box, num))
        else:
            tm.assert_equal(expected, op(bool_box, num))
        tm.assert_equal(expected, op(num, bool_box))

This looks sloppy, and not really readable, any thoughts?

Copy link
Member

@jorisvandenbossche jorisvandenbossche Feb 24, 2020

Choose a reason for hiding this comment

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

That looks fine to me in general (have some comments on some details, but that will be easier to add when this is in the diff)


bool_box = [True]
expected = [op(num, num)]

bool_box = tm.box_expected(bool_box, box_with_array)
expected = tm.box_expected(expected, box_with_array)

tm.assert_equal(expected, op(bool_box, num))
tm.assert_equal(expected, op(num, bool_box))
Copy link
Member

@jbrockmendel jbrockmendel Feb 19, 2020

Choose a reason for hiding this comment

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

nitpick, the pattern we usually use is:

bool_box = [True]
expected = [op(num, num)]

bool_box = tm.box_expected(...
expected = tm.box_expected(...

result = op(bool_box, num)
tm.assert_equal(...
result = op(num, bool_box)
tm.assert_equal(...