BUG: Fix issue with apply on empty DataFrame #28213

dsaxton · 2019-08-29T04:09:10Z

Fixes a bug where the return value for certain functions was hard-coded as np.nan when operating on an empty DataFrame.

Before

>>> import numpy as np
>>> import pandas as pd
>>> df = pd.DataFrame(columns=["a", "b", "c"])
>>> df.apply(np.sum)
a   NaN
b   NaN
c   NaN
dtype: float64
>>> df.apply(np.prod)
a   NaN
b   NaN
c   NaN
dtype: float64

After

>>> import numpy as np
>>> import pandas as pd
>>> df = pd.DataFrame(columns=["a", "b", "c"])
>>> df.apply(np.sum)
a    0.0
b    0.0
c    0.0
dtype: float64
>>> df.apply(np.prod)
a    1.0
b    1.0
c    1.0
dtype: float64

Edit: Closes #28202 and closes #21959 after cb68153. The issue was that the arguments of self.f were already unpacked here:

pandas/pandas/core/apply.py

Line 112 in cb68153

return func(x, *args, **kwds)

and then we tried to do this again inside apply_empty_result which was raising an error and causing the unusual output.

pep8speaks · 2019-08-29T04:09:12Z

Hello @dsaxton! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-09-12 13:19:32 UTC

pandas/core/apply.py

jbrockmendel · 2019-08-29T17:54:09Z

needs a test

dsaxton · 2019-08-29T23:06:05Z

needs a test

@jbrockmendel Added a few tests that appear to be failing on master. There are some existing tests failing on this branch that I still need to work through.

TomAugspurger · 2019-08-30T13:28:49Z

Also closes #28202

Does this close any other issues?

Can you add a release note in 1.0.0.rst?

pandas/tests/frame/test_apply.py

dsaxton · 2019-08-30T22:22:40Z

Also closes #28202

Does this close any other issues?

Can you add a release note in 1.0.0.rst?

I believe it also closes #21959, which was still an issue at least as of 0.25.1 (not sure about master)

pandas/tests/frame/test_apply.py

This reverts commit 1993c7c.

pandas/core/apply.py

jreback · 2019-09-02T21:36:47Z

doc/source/whatsnew/v1.0.0.rst

@@ -84,6 +84,7 @@ Performance improvements
 Bug fixes
 ~~~~~~~~~

+- Bug in :meth:`DataFrame.apply` that caused incorrect output with empty :class:`DataFrame` (:issue:`28202`, :issue:`21959`)


this is specific to np.* and .nunique()? if so can you be more specific

I think it'd apply to any reduction whose output isn't empty when the input is, should I say something to that effect?

doc/source/whatsnew/v1.0.0.rst

jreback · 2019-09-02T21:37:14Z

pandas/core/apply.py

                reduce = not isinstance(r, Series)
            except Exception:
                pass

        if reduce:
-            return self.obj._constructor_sliced(np.nan, index=self.agg_axis)
+            if len(self.agg_axis):
+                r = self.f(Series([]))


pass args & kwargs

jreback · 2019-09-02T21:39:16Z

pandas/core/apply.py

                reduce = not isinstance(r, Series)
            except Exception:
                pass

        if reduce:
-            return self.obj._constructor_sliced(np.nan, index=self.agg_axis)
+            if len(self.agg_axis):
+                r = self.f(Series([]))


we already are trying to reduce above (line 208), why are you calling the function again? does this hit the Except?

I did that in case reduce was already True at line 206, so that the try block wouldn't have been executed

i am still puzzled why you can not pass args/kwargs

I think what's happening is the function self.f is getting curried around here:

pandas/pandas/core/apply.py

Line 109 in cb68153

if (kwds or args) and not isinstance(func, (np.ufunc, str)):

so when we pass the arguments again we get an error (this is from the df.nunique() test):

> r = self.f(Series([]), *self.args, **self.kwds) E TypeError: f() got an unexpected keyword argument 'dropna'

because at that point f only takes a single argument. I could imagine there could end up being a problem if this currying doesn't happen, so there's probably a hidden corner case that just isn't covered by the existing tests.

Was confused by this to but I think this makes sense. I suppose this was hitting the except before due to a TypeError for wrong number of arguments?

This reverts commit 9442a04.

TomAugspurger

Was the discussion on passing through args & kwargs resolved?

dsaxton · 2019-09-12T21:29:45Z

Was the discussion on passing through args & kwargs resolved?

If I turn off the try / catching completely to try to surface any other issues, then adding back the argument unpacking only causes test_nunique_empty to fail among the tests in test_apply.py (three failed tests instead of the two mentioned above), so I would say we shouldn't be passing them.

TomAugspurger · 2019-09-13T21:23:12Z

Sounds good. I think we're good to merge then?

dsaxton · 2019-09-13T21:47:10Z

Sounds good. I think we're good to merge then?

I'd say so unless @jreback can foresee any other potential problems

TomAugspurger · 2019-09-19T21:19:24Z

OK. Let's give it a few more days in case.

jreback · 2019-09-19T23:27:41Z

lgtm but need to edit the top of the PR to add closes on the other issue number

dsaxton · 2019-09-19T23:47:03Z

lgtm but need to edit the top of the PR to add closes on the other issue number

Done, thanks for the review!

WillAyd · 2019-09-20T03:36:13Z

Thanks @dsaxton

Use try / except on empty Series

444209d

Fix PEP8

b749cca

mroeschke reviewed Aug 29, 2019

View reviewed changes

pandas/core/apply.py Outdated Show resolved Hide resolved

Daniel Saxton added 2 commits August 29, 2019 06:53

Remove try / except

d9b620b

Only apply on non-empty agg_axis

a82479a

Add tests

aa4b33b

Daniel Saxton added 3 commits August 29, 2019 19:12

Move condition

61f432f

Don't unpack args twice

cb68153

Add nunique test

f4c7a97

TomAugspurger added the Apply Apply, Aggregate, Transform, Map label Aug 30, 2019

TomAugspurger added this to the 1.0 milestone Aug 30, 2019

jreback requested changes Aug 30, 2019

View reviewed changes

pandas/tests/frame/test_apply.py Outdated Show resolved Hide resolved

Daniel Saxton added 2 commits August 30, 2019 17:29

Add to release notes

633a6b8

Parametrize tests

1993c7c

mroeschke reviewed Aug 30, 2019

View reviewed changes

pandas/tests/frame/test_apply.py Outdated Show resolved Hide resolved

Daniel Saxton added 5 commits August 30, 2019 19:15

Revert "Parametrize tests"

a0c09eb

This reverts commit 1993c7c.

Parametrize tests

51de6ef

Use getattr

91ef657

Add nunique test

4a9ea0b

Set index

42b2209

jreback requested changes Sep 2, 2019

View reviewed changes

Daniel Saxton added 4 commits September 2, 2019 17:24

Move note under reshaping

de0e1dd

Merge branch 'master' into apply-fix

6b98060

Use should_reduce

9d6ae49

Fix merge

2067c1a

Daniel Saxton added 3 commits September 9, 2019 18:14

Narrow exception catching

9442a04

Merge branch 'master' into apply-fix

20a3199

Revert "Narrow exception catching"

f3077aa

This reverts commit 9442a04.

TomAugspurger reviewed Sep 12, 2019

View reviewed changes

jreback approved these changes Sep 19, 2019

View reviewed changes

WillAyd approved these changes Sep 20, 2019

View reviewed changes

WillAyd merged commit 95edcf1 into pandas-dev:master Sep 20, 2019

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

BUG: Fix issue with apply on empty DataFrame (pandas-dev#28213)

1c1eb80

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

BUG: Fix issue with apply on empty DataFrame (pandas-dev#28213)

d240554

dsaxton deleted the apply-fix branch April 9, 2020 02:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Fix issue with apply on empty DataFrame #28213

BUG: Fix issue with apply on empty DataFrame #28213

dsaxton commented Aug 29, 2019 •

edited

Loading

pep8speaks commented Aug 29, 2019 •

edited

Loading

jbrockmendel commented Aug 29, 2019

dsaxton commented Aug 29, 2019

TomAugspurger commented Aug 30, 2019

dsaxton commented Aug 30, 2019

jreback Sep 2, 2019

dsaxton Sep 2, 2019

jreback Sep 2, 2019

jreback Sep 2, 2019

dsaxton Sep 2, 2019

jreback Sep 13, 2019

dsaxton Sep 13, 2019 •

edited

Loading

WillAyd Sep 19, 2019

TomAugspurger left a comment

dsaxton commented Sep 12, 2019

TomAugspurger commented Sep 13, 2019

dsaxton commented Sep 13, 2019

TomAugspurger commented Sep 19, 2019

jreback commented Sep 19, 2019

dsaxton commented Sep 19, 2019

WillAyd commented Sep 20, 2019

BUG: Fix issue with apply on empty DataFrame #28213

BUG: Fix issue with apply on empty DataFrame #28213

Conversation

dsaxton commented Aug 29, 2019 • edited Loading

pep8speaks commented Aug 29, 2019 • edited Loading

Comment last updated at 2019-09-12 13:19:32 UTC

jbrockmendel commented Aug 29, 2019

dsaxton commented Aug 29, 2019

TomAugspurger commented Aug 30, 2019

dsaxton commented Aug 30, 2019

jreback Sep 2, 2019

Choose a reason for hiding this comment

dsaxton Sep 2, 2019

Choose a reason for hiding this comment

jreback Sep 2, 2019

Choose a reason for hiding this comment

jreback Sep 2, 2019

Choose a reason for hiding this comment

dsaxton Sep 2, 2019

Choose a reason for hiding this comment

jreback Sep 13, 2019

Choose a reason for hiding this comment

dsaxton Sep 13, 2019 • edited Loading

Choose a reason for hiding this comment

WillAyd Sep 19, 2019

Choose a reason for hiding this comment

TomAugspurger left a comment

Choose a reason for hiding this comment

dsaxton commented Sep 12, 2019

TomAugspurger commented Sep 13, 2019

dsaxton commented Sep 13, 2019

TomAugspurger commented Sep 19, 2019

jreback commented Sep 19, 2019

dsaxton commented Sep 19, 2019

WillAyd commented Sep 20, 2019

dsaxton commented Aug 29, 2019 •

edited

Loading

pep8speaks commented Aug 29, 2019 •

edited

Loading

dsaxton Sep 13, 2019 •

edited

Loading