Skip to content
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

Fix upstream #7538

Merged
merged 1 commit into from Apr 12, 2021
Merged

Fix upstream #7538

merged 1 commit into from Apr 12, 2021

Conversation

jsignell
Copy link
Member

@jsignell jsignell commented Apr 7, 2021

Something might have changed in pandas groupby code, but this seems like a good solution for cases where there might be only one partition in a tree reduction. I didn't check to see if there are more cases where this pattern needs updating.

@jsignell
Copy link
Member Author

jsignell commented Apr 8, 2021

Good with you @jrbourbeau? I grepped around for groupby(level=0) and reset_index(level=1 and I don't see anywhere else that would have the same issue. So I think this can be merged and doesn't need to be treated as an indicator of a larger issue.

@jsignell jsignell merged commit 09862ed into dask:main Apr 12, 2021
@jsignell jsignell deleted the fix-upstream branch April 12, 2021 13:57
@@ -6516,7 +6516,7 @@ def idxmaxmin_row(x, fn=None, skipna=True):


def idxmaxmin_combine(x, fn=None, skipna=True):
if len(x) == 0:
if len(x) <= 1:
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for not getting to getting to this sooner. I think the len(x) == 0 case was to handle empty DataFrames. Do you know if our test suite covers the len(x) == 1 case? I'm somewhat surprised this works if we just pass a non-empty DataFrame through

Copy link
Member Author

Choose a reason for hiding this comment

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

I think in the tests that were failing, the issue was that in the agg step, len(x) was occasionally == 1. This happened when split_every=2 and the total number of partitions was 3, so sometimes there was only one item in x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upstream failures with idxmaxmin
2 participants