Skip to content

TST: Add tests for num-bool coercion #28966

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

Closed
wants to merge 1 commit into from
Closed

TST: Add tests for num-bool coercion #28966

wants to merge 1 commit into from

Conversation

kgoehner
Copy link

@kgoehner kgoehner commented Oct 14, 2019

Boolean dtypes in both Series and DataFrames should be coerced
to the other operand's dtype.

@WillAyd WillAyd added the Testing pandas testing functions or related to the test suite label Oct 14, 2019
@WillAyd
Copy link
Member

WillAyd commented Oct 14, 2019

@jbrockmendel was this perhaps fixed by some of the _maybe_promote work you've been doing?

@jbrockmendel
Copy link
Member

was this perhaps fixed by some of the _maybe_promote work you've been doing?

Doubtful. I don't think maybe_promote is used anywhere in/around ops.

@Kazz47 would the same test be applicable for Index classes? Can you parametrize the test over [Index, Series, DataFrame] and put it somewhere in tests.arithmetic?

@kgoehner
Copy link
Author

@jbrockmendel some of the operators (multiply and floordiv) aren't supported by Index. Should these be skipped if we see a TypeError for some operators? If so I can update the PR. Otherwise, do you have a more graceful way of handling this?

    @pytest.mark.parametrize("container_cls_assert", [(pd.Index, tm.assert_index_equal),
                                                      (pd.Series, tm.assert_series_equal),
                                                      (pd.DataFrame, tm.assert_frame_equal)])
    @pytest.mark.parametrize("num", [1.0, 1])
    @pytest.mark.parametrize(
        "op", [operator.add, operator.mul, operator.floordiv, operator.sub]
    )
    def test_array_like_bool_and_num_op_coerce(self, container_cls_assert, num, op):
        # GH 18549
        container_cls = container_cls_assert[0]
        container_assert = container_cls_assert[1]
        expected = container_cls([op(num, num)])
        bool_array = container_cls([True])
        container_assert(expected, op(bool_array, num))
        container_assert(expected, op(num, bool_array))

@jbrockmendel
Copy link
Member

some of the operators (multiply and floordiv) aren't supported by Index

If the array you're passing to Index is all numeric, you should get Int64Index or Float64Index, so you'll be OK.

You can use tm.assert_equal instead of container_cls_assert.

In tests.arithmetic we use a box or box_with_array fixture along with calls to tm.box_expected(obj, box) to parametrize tests across classes. If you can adhere to that pattern that'd be ideal

@kgoehner
Copy link
Author

If the array you're passing to Index is all numeric, you should get Int64Index or Float64Index, so you'll be OK.

Since we are passing a bool we don't get Int64Index or Float64Index and the numeric operators are being disabled.

Index._add_numeric_methods_disabled()

Latest version:

class TestNumericArraylikeArithmeticWithBool:
    @pytest.mark.parametrize("num", [1.0, 1])
    @pytest.mark.parametrize(
        "op", [operator.add, operator.mul, operator.floordiv, operator.sub]
    )
    def test_array_like_bool_and_num_op_coerce(self, num, op, box_with_array):
        # GH 18549
        expected = [op(num, num)]
        expected = tm.box_expected(expected, box_with_array)
        bool_box = tm.box_expected([True], box_with_array)
        tm.assert_equal(expected, op(bool_box, num))
        tm.assert_equal(expected, op(num, bool_box))

Boolean dtypes in both Series and DataFrames should be coerced
to the other operand's dtype.
expected = [op(num, num)]
expected = tm.box_expected(expected, box_with_array)
bool_box = tm.box_expected([True], box_with_array)
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

rather than catching you should return on the operators that we don't want to test, making this more explicit

@WillAyd
Copy link
Member

WillAyd commented Nov 7, 2019

@Kazz47 is this still active? Can you merge master and address comment?

@WillAyd
Copy link
Member

WillAyd commented Dec 17, 2019

Nice PR but needs rebase and I think has gone stale. Ping if you'd like to pick this back up

@WillAyd WillAyd closed this Dec 17, 2019
@ShaharNaveh ShaharNaveh mentioned this pull request Feb 1, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: boolean frames multiplied by floats have dtypes=object
5 participants