pandas icon indicating copy to clipboard operation
pandas copied to clipboard

REGR: Behavior change with empty apply in pandas 1.3.0rc1

Open aberres opened this issue 4 years ago • 18 comments

Problem description

The following (toy) snippet worked with 1.2:

df = pd.DataFrame(columns=["a", "b"])
df["a"] = df.apply(lambda x: x["a"], axis=1)

With 1.3 it fails with ValueError: Columns must be same length as key

Technically this is correct - the apply on an empty frame returns the empty frame so things do not really match.

Expected Output

It still works? Just reporting it here if this is an unintended change. Maybe I missed it, but I did not see this mentioned in the changelog.

The fix is to only call apply when the frame is not empty I guess? I stumbled upon this one when running our test suite.

aberres avatar Jun 14 '21 10:06 aberres

Thanks @aberres for the report

Just reporting it here if this is an unintended change. Maybe I missed it, but I did not see this mentioned in the changelog.

first bad commit: [caf81fac62071e9582379ea8c8a9cf40396f2fe7] BUG: DataFrame.setitem not raising ValueError when rhs is df and has wrong number of columns (#39341)

cc @phofl

simonjayhawkins avatar Jun 14 '21 11:06 simonjayhawkins

@simonjayhawkins I've relabeled, since this is an indexing issue, not related to apply.

The RHS is

Empty DataFrame
Columns: [a, b]
Index: []

This is basically the case we have changed the behavior for.

df = pd.DataFrame(columns=["a", "b"])
rhs = pd.DataFrame(columns=["a", "b"])
df["a"] = rhs

I think this behaves now as expected.

@aberres The note you are looking for is the one for #38604 in the whatsnew

Edit: The idea behind this is, that the number of columns has to match. Previously we assigned always the first column and silently dropped the rest.

phofl avatar Jun 14 '21 12:06 phofl

@aberres The note you are looking for is the one for #38604 in the whatsnew

Thanks, makes sense.

Not sure if this happens a lot out in the wild - but maybe this case should be allowed for empty columns? I addd if not df.empty in my code which worked fine - as expected.

aberres avatar Jun 14 '21 12:06 aberres

Don't think behavior based on data is desirable. I am not sure what you want to achieve here if this works for empty dfs. When you know, that your DataFrame is empty why bothering with the setitem call? If it is not empty, this will raise even if we would allow empty frames

@jbrockmendel thoughts?

phofl avatar Jun 14 '21 13:06 phofl

Hm maybe I was wrong above and this is an apply issue.

df = pd.DataFrame([[1, 2]], columns=["a", "b"])
df.apply(lambda x: x["a"], axis=1)
0    1
dtype: int64


df = pd.DataFrame(columns=["a", "b"])
df.apply(lambda x: x["a"], axis=1)

Empty DataFrame
Columns: [a, b]
Index: []

Looks inconsistent, but I am not familiar enough with apply to asses this.

Nevertheless I am against catching the incosistency here in setitem. You can get into all sorts of trouble, if you expect a Series but receive a DataFrame.

phofl avatar Jun 14 '21 13:06 phofl

If it is not empty, this will raise even if we would allow empty frames

Yeah, as you noticed in the "not empty case" only a single column is returned and the assignment works fine.

Technically it is not wrong to raise in the empty case. It is just a change in behavior which might or might not cause problems.

aberres avatar Jun 14 '21 13:06 aberres

Nevertheless I am against catching the incosistency here in setitem. You can get into all sorts of trouble

Agreed

df = pd.DataFrame(columns=["a", "b"])
df.apply(lambda x: x["a"], axis=1)

Empty DataFrame
Columns: [a, b]
Index: []

Looks inconsistent, but I am not familiar enough with apply to asses this.

Yah seems like we should get back and empty Series right?

jbrockmendel avatar Jun 14 '21 16:06 jbrockmendel

Yah seems like we should get back and empty Series right?

yes would have expected that as well

phofl avatar Jun 14 '21 16:06 phofl

I don't know how we can determine what we should get back when we have nothing to go on. Consider:

df = pd.DataFrame(columns=["a", "b"])
df.apply(lambda x: x["a"], axis=1)
df.apply(lambda x: x[["a", "b"]], axis=1)

How can we tell the difference between these internally? It seems to me the only options are to either raise directly or return the df as-is.

Edit: But agreed this is not an issue with setitem. While I see this behavior as undesired, I don't see a way to avoid it.

rhshadrach avatar Jun 25 '21 20:06 rhshadrach

Looking at this again, pandas.core.apply.FrameApply.apply_empty_result is attempting to pass an empty Series to discern the shape of the output. However because that empty Series is not able to have index values, the lambda in this issue fail and this method fallsback to just returning the entire object itself.

If we were able to pass in an empty Series with the correct index values ("a" and "b"), we could differentiate between these two. However as far as I know, that currently is not possible.

rhshadrach avatar Jul 18 '21 16:07 rhshadrach

I am also experiencing the same issue. Example below.

df = pd.DataFrame(columns=["a", "b"])
df['a'] = df.apply(lambda x: x["a"], axis=1)

I would expect 'x' in this case to be an empty Series, but instead it is returning an empty DataFrame. When as Any ideas on expected resolution?

jordantshaw avatar Jul 27 '21 17:07 jordantshaw

changing milestone to 1.3.5

simonjayhawkins avatar Oct 16 '21 19:10 simonjayhawkins

Just reported a related issue with apply swallowing exceptions: https://github.com/pandas-dev/pandas/issues/44031

apply explicitly ignores all exceptions when processing an empty DataFrame. This leads to apply returning an empty DataFrame, which is an inconsistency, as it was already pointed out. https://github.com/pandas-dev/pandas/blob/22de58e63f9271d4ddb2bf49d008c5a9550c5cc4/pandas/core/apply.py#L779-L782

In my opinion, this inconsistency is actually caused by another inconsistency:

  • When DataFrame has rows, apply(..., axis=1) takes each row as Series.
  • However, when DataFrame is empty, we pass empty series.

The issue is that semantics are different between these two cases. In the former, keys exist and are the same as the DataFrame columns. In the latter, keys don't exist - we get a different data structure.

Option 1 We don't swallow exceptions in apply on empty DataFrame. This would be equivalent to the behaviour on non-empty DataFrame. We let the user to handle empty Series vs non-empty Series in their apply function.

Option 2 In pandas, a Series with keys and no data is not possible. To avoid this inconsistency then we could pass a series with keys and None as values. Semantically this would be closer to passing a Series with no data than the current solution with empty series:

  • keys a present, no exception on indexing raised
  • indexing returns None, should be handled by the user.

Thoughts?

gshaikov avatar Oct 18 '21 12:10 gshaikov

Hi @rhshadrach any new comments on the above? Happy to make an PR is this sounds reasonable.

gshaikov avatar Nov 07 '21 15:11 gshaikov

@gshaikov - Option 2 doesn't seem viable to me. There is no way for a user to differentiate between a Series of all None values and an empty Series. I would also describe that user experience as being "quirky".

Option 1 does seem better than the current behavior - it would allow for the user to handle an empty DataFrame as they see fit. However, it's tempting to think of apply as "for loop over the rows/columns", and it violates this viewpoint as an empty frame should result in calling the UDF 0 times (which is currently violated in today's implementation too!). With this, it doesn't seem like a clear win to me and I'd like to hear what others might think.

rhshadrach avatar Nov 19 '21 22:11 rhshadrach

Looking at this issue from purely a backport fix perspective, I doesn't appear that we have any solutions here (for 1.3.x).

Changing the behavior of apply would not be suitable for a backport.

For backport, we would either need to:

  1. revert the change to setitem that caused the regression.
  2. catch the apply inconsistency in setitem

I think option 2 has been ruled out https://github.com/pandas-dev/pandas/issues/41997#issuecomment-860671005 and https://github.com/pandas-dev/pandas/issues/41997#issuecomment-860833386

I think option 1 is undesirable (especially late in the 1.3.x branch) since the change was a bugfix and is now correct behavior. https://github.com/pandas-dev/pandas/issues/41997#issuecomment-860629747

I propose we remove this from the 1.3.5 milestone.

simonjayhawkins avatar Nov 27 '21 12:11 simonjayhawkins

removing issue from 1.3.x milestone. Any changes to address the apply issue would not be backported.

simonjayhawkins avatar Nov 28 '21 10:11 simonjayhawkins

As I mentioned in #47966 this behavior was previously documented and at least should be documented again.

emsi avatar Aug 05 '22 07:08 emsi