Skip to content

added errorchecking for get_group #1

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

Closed
wants to merge 5 commits into from
Closed

Conversation

WillySong
Copy link

@WillySong WillySong commented Mar 9, 2020

I can't believe it took me all night to understand and debug this.
I have no clue how to do the rest of this below.

I can't believe it took me all night to understand and debug this.
@WillySong WillySong self-assigned this Mar 9, 2020
@WillySong WillySong requested review from hoangbn and letapxad March 9, 2020 13:03
@WillySong WillySong changed the title errorchecking for get_group added errorchecking for get_group Mar 9, 2020
@WillySong
Copy link
Author

For reference, the error truly exists in how we use _get_indices. I cannot read nor debug whatever god awful C24 lambda code they wrote for that function, but it returns what we see as a valid input in our get_group function. I have added error checking this case (as per the issue). Please verify this change works (two test cases provided if you run the file) and then I'll build some more test cases.

@letapxad
Copy link

For reference, the error truly exists in how we use _get_indices. I cannot read nor debug whatever god awful C24 lambda code they wrote for that function, but it returns what we see as a valid input in our get_group function. I have added error checking this case (as per the issue). Please verify this change works (two test cases provided if you run the file) and then I'll build some more test cases.

I can agree about the _get_indices implementation

@WillySong WillySong requested review from julesyan and removed request for hoangbn March 11, 2020 03:15
@julesyan
Copy link

Tests dont fail on the original code, need more cases.
Fix does not complete original issue after testing in dev environment.

@julesyan julesyan closed this Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dataframe.groupby incorrect with multiindex and None value
3 participants