BUG: _can_use_numexpr fails when passed large Series

[ccharlesgb]

This fixes a regression introduced in #27145 where _can_use_numexpr would fail if passed a Series and not a DataFrame. I decided not to use run_arithmetic in the test_suite because there is a separate issue when running floordiv that is out of the scope of the fix. I will open a separate issue in improving the test coverage of "test_expressions.py" as it currently does not check any of the arguments you can pass to the operators, such as 'axis', 'level' and 'fill_value'.

• closes Operators between DataFrame and Series fail on large dataframes #27636
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[ccharlesgb]

There was some CI failures (Looked like some unrelated http exceptions so I pulled the latest commits in from master to see if that fixes it).

I have added the xfail for axis=1 on floordiv and made the whatsnew entry more specific now which should now cover all comments.

[jbrockmendel]

I think the CI error is unrelated. Can you rebase and see if it resolves itself?

[ccharlesgb]

I think the CI error is unrelated. Can you rebase and see if it resolves itself?

That has fixed it. @WillAyd are you comfortable with the changes?

[WillAyd]

What does this do?

[ccharlesgb]

There is an overhead in using numexpr for computations that makes it not worth using for a low number of computations. The values of this defaults to: 10,000. In this test suite we are trying to verify that numexpr is being invoked correctly however the function _can_use_numexpr checks that the objects operated on are sufficient size:

def _can_use_numexpr(op, op_str, a, b, dtype_check):
    """ return a boolean if we WILL be using numexpr """
    if op_str is not None:
        # required min elements (otherwise we are adding overhead)
        if np.prod(a.shape) > _MIN_ELEMENTS:
            # further dtype checks to check compatibility may return True
    return False

We can therefore run the test suite exclusively on objects where the number of elements > 10,000 or set the min elements to zero to always try and use numexpr.

This is what is meant by a "large DataFrame". Anything where the size of the objects is large enough to warrant using numexpr fails because of the regression, anything that is smaller than the threshold will succeed because numexpr evaluation will not even be considered.

[TomAugspurger]

Hmm is affecting global state, right? I worry about this leaking into other tests...

Can you use the monkeypatch fixture, and do this setting with (IIRC) monkeypatch.setattr? That way it'll be undone when the test exits.

[ccharlesgb]

It is but this is handled in the test classes teardown method on line 57 so the global state won't be affected permanently. I can move my test out of the class and handle this with monkeypatch or we can keep the setup/teardown framework already implemented in the TestExpressions class.

[TomAugspurger]

Thanks, I wasn't familiar with these tests.

[ccharlesgb]

Hi @WillAyd is the Whatsnew entry ok now?

[TomAugspurger]

Looks close, one question about the test.

[TomAugspurger]

Hmm is affecting global state, right? I worry about this leaking into other tests...

Can you use the monkeypatch fixture, and do this setting with (IIRC) monkeypatch.setattr? That way it'll be undone when the test exits.

[TomAugspurger]

FYI, can you just merge master & do a regular push, rather than a force push? We don't always get notifications on force pushes.

[jbrockmendel]

This should also close #27853 @ccharlesgb can you make sure the use case there is covered by the new tests?

[ccharlesgb]

This should also close #27853 @ccharlesgb can you make sure the use case there is covered by the new tests?

@jbrockmendel yes this case is covered in the test as long as divide/div and truediv are all still aliases of each other?

[ccharlesgb]

FYI, can you just merge master & do a regular push, rather than a force push? We don't always get notifications on force pushes.

@jbrockmendel Had to rebase as the CI was failing again. But I think everything is addressed now.

[TomAugspurger]

Thanks @ccharlesgb!