Skip to content

Fix issue #29837: added test case for aggregation with isnan #35039

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jun 29, 2020
Merged

Fix issue #29837: added test case for aggregation with isnan #35039

merged 6 commits into from
Jun 29, 2020

Conversation

biddwan09
Copy link
Contributor

@biddwan09 biddwan09 commented Jun 28, 2020 •
edited
Loading

@pep8speaks
Copy link

pep8speaks commented Jun 28, 2020 •
edited
Loading

Hello @biddwan09! 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 2020-06-29 04:30:55 UTC

@simonjayhawkins
Copy link
Member

@biddwan09 thanks for the PR. FYI Linux py37_np_dev test failures unrelated, xref #35041

@simonjayhawkins simonjayhawkins added Categorical Categorical Data Type Groupby Testing pandas testing functions or related to the test suite labels Jun 28, 2020
simonjayhawkins
simonjayhawkins reviewed Jun 28, 2020
View reviewed changes
Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @biddwan09 generally lgtm.

@simonjayhawkins simonjayhawkins added this to the 1.1 milestone Jun 28, 2020
@biddwan09 @simonjayhawkins
Update pandas/tests/groupby/aggregate/test_aggregate.py
b2dcb9d 
changed pd.testing to tm

Co-authored-by: Simon Hawkins <[email protected]>
@biddwan09
Copy link
Contributor Author

Hi @simonjayhawkins let me know if any other change is required

@simonjayhawkins simonjayhawkins merged commit f6fb257 into pandas-dev:master Jun 29, 2020
@simonjayhawkins
Copy link
Member

Thanks @biddwan09

@biddwan09 biddwan09 deleted the fix-#29837 branch June 29, 2020 07:46
jreback
jreback reviewed Jun 29, 2020
View reviewed changes
pandas/tests/groupby/aggregate/test_aggregate.py


def test_aggregate_categorical_with_isnan():
# GH 29837
Copy link
Contributor

Choose a reason for hiding this comment

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

prob should have been in test_category.py

Copy link
Member

Choose a reason for hiding this comment

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

There are two other tests for categorical in that file, test_agg_cython_category_not_implemented_fallback and test_aggregate_categorical_lost_index. move those as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah prob

Copy link
Member

Choose a reason for hiding this comment

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

moved in #35052

@simonjayhawkins simonjayhawkins mentioned this pull request Jun 29, 2020
Merged
@biddwan09 biddwan09 restored the fix-#29837 branch October 13, 2020 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback left review comments

@simonjayhawkins simonjayhawkins simonjayhawkins left review comments

Assignees
No one assigned
Labels
Categorical Categorical Data Type Groupby Testing pandas testing functions or related to the test suite
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.

groupby() drops categorical columns when aggregating with isna()
4 participants
@biddwan09 @pep8speaks @simonjayhawkins @jreback