Pandas BUG: test_finalize.py tests failing on master with pytest-randomly

On Ubuntu18 im seeing a variable number of tests in test_binops failing with empty .attrs

cc @TomAugspurger

Comment From: TomAugspurger

This is on CI or just locally? Any tracebacks you can share?

Comment From: jbrockmendel

Just locally

____________________________________ test_binops[rmul-args0-right] _____________________________________

args = (1, 0    1
dtype: int64), annotate = 'right'
all_arithmetic_functions = <function rmul at 0x7f4a5c3df400>

    @pytest.mark.parametrize("annotate", ["left", "right", "both"])
    @pytest.mark.parametrize(
        "args",
        [
            (1, pd.Series([1])),
            (1, pd.DataFrame({"A": [1]})),
            (pd.Series([1]), 1),
            (pd.DataFrame({"A": [1]}), 1),
            (pd.Series([1]), pd.Series([1])),
            (pd.DataFrame({"A": [1]}), pd.DataFrame({"A": [1]})),
            (pd.Series([1]), pd.DataFrame({"A": [1]})),
            (pd.DataFrame({"A": [1]}), pd.Series([1])),
        ],
    )
    def test_binops(args, annotate, all_arithmetic_functions):
        # This generates 326 tests... Is that needed?
        left, right = args
        if annotate == "both" and isinstance(left, int) or isinstance(right, int):
            return

        if isinstance(left, pd.DataFrame) or isinstance(right, pd.DataFrame):
            pytest.xfail(reason="not implemented")

        if annotate in {"left", "both"} and not isinstance(left, int):
            left.attrs = {"a": 1}
        if annotate in {"left", "both"} and not isinstance(right, int):
            right.attrs = {"a": 1}

        result = all_arithmetic_functions(left, right)
>       assert result.attrs == {"a": 1}
E       AssertionError: assert {} == {'a': 1}
E         Right contains 1 more item:
E         {'a': 1}
E         Use -v to get the full diff

Comment From: jbrockmendel

It looks like i have (had) pytest-randomly installed on this machine. Removing it fixes these failures. So it looks like there's some global state somewhere

Comment From: jbrockmendel

@TomAugspurger i thought i pinged on this last week but can't find it. I think there may be a simple solution here, need you to confirm if it is correct.

Am I right in thinking the following should always hold:

def test_binops(args, annotate, all_arithmetic_functions):
    # This generates 326 tests... Is that needed?
    left, right = args
    if annotate == "both" and isinstance(left, int) or isinstance(right, int):
        return

+    if isinstance(left, (pd.Series, pd.DataFrame)):
+        assert left.attrs == {}
+    if isinstance(right, (pd.Series, pd.DataFrame)):
+        assert right.attrs == {}

Comment From: TomAugspurger

It seems a bit strange to make an assertion about the inputs to the test, no? But I guess the issue is that they're being unexpectedly mutated somehow?

Comment From: jbrockmendel

But I guess the issue is that they're being unexpectedly mutated somehow?

exactly. the same Series/DataFrame objects are being re-used across the parametrized tests, so when the attrs are set in the lines below, this assumption is invalidated in the following tests.

so if its correct that these assertions should hold, then we can just clear the attrs at the beginning of each test and be fine

Comment From: jbrockmendel

related, i think a possible typo?

` if annotate in {"left", "both"} and not isinstance(left, int): left.attrs = {"a": 1} - if annotate in {"left", "both"} and not isinstance(right, int): + if annotate in {"right", "both"} and not isinstance(right, int): right.attrs = {"a": 1}

Comment From: jbrockmendel

@TomAugspurger gentle ping. if im right about the expected behavior, ive got a branch ready to push

Comment From: TomAugspurger

Ah I see, you're using the asserts to show the expected behavior. The diff would be to set the attrs (and maybe make a copy too?).

If so, yes I agree, and that does seem like a typo on the second if annotate in ....

Comment From: jbrockmendel

Well I take back the part about the branch being ready... a few weeks ago I thought I had this working, but now clearing out the attrs at the start of the test breaks 42 tests (regardless of whether I change the "left" -> "right")

Comment From: k-ishizaka

This test might try to assert result.attrs == {"a": 1} even when result = add(left, right) with left.attrs={} and right.attrs={"a": 1}. Should it be true or false?