Skip to content

BUG: SeriesGroupby.nunique raises an IndexError on empty Series #12553 #12560

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 1 commit into from
Closed

Conversation

thanasis2028
Copy link

@thanasis2028 thanasis2028 commented Mar 8, 2016

Closes #12553
xref #13239
Made a new pull request to clean up from #12557

@jreback
Copy link
Contributor

jreback commented Mar 8, 2016

needs tests

@jreback jreback changed the title BUG: Fix for issue #12553 BUG: SeriesGroupby.nunique raises an IndexError on empty Series #12553 Mar 8, 2016
@thanasis2028
Copy link
Author

I'm sry what am I supposed to do? Do you want me to run nosetests pandas or make new tests?

@jreback
Copy link
Contributor

jreback commented Mar 8, 2016

new tests, see contributing docs here

you have a failing case, so create a test which fails w/o your fix (put the tests in test_groupby.py)

then your fix should make it pass

@jreback
Copy link
Contributor

jreback commented May 7, 2016

can you rebase / update & add whatsnew for 0.18.2

@jreback
Copy link
Contributor

jreback commented May 14, 2016

can you rebase/update

@codecov-io
Copy link

codecov-io commented May 14, 2016

Current coverage is 84.12%

Merging #12560 into master will decrease coverage by 0.09%

@@             master     #12560   diff @@
==========================================
  Files           138        138          
  Lines         50713      50387   -326   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          42715      42385   -330   
- Misses         7998       8002     +4   
  Partials          0          0          

Powered by Codecov. Last updated by fcd73ad...ee6d924

@thanasis2028
Copy link
Author

Done, sry for 2 commits but when I try to rebase everything gets messed up...

@@ -2833,6 +2833,10 @@ def true_and_notnull(x, *args, **kwargs):
def nunique(self, dropna=True):
""" Returns number of unique elements in the group """
ids, _, _ = self.grouper.group_info

if len(ids) == 0: # bugfix for 12553
return Series([])
Copy link
Contributor

Choose a reason for hiding this comment

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

need name=self.name

@jreback
Copy link
Contributor

jreback commented May 31, 2016

can you rebase / update

Author:    Thanasis Katsios <[email protected]>
group = series.groupby(level=0)
result = group.nunique()
expected = Series(name='foo')
assert_series_equal(result, expected)
Copy link
Member

Choose a reason for hiding this comment

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

Can you put this test inside the TestGroupBy class? Eg after test_nunique_with_object at Line 6404 (https://github.com/thanasis2028/pandas/blob/744e537c3c73ecb30a62a7707db138b7f4dafcd8/pandas/tests/test_groupby.py#L6400)

@jorisvandenbossche
Copy link
Member

@thanasis2028 Your change is causing an error in test_resample_empty_series (probably resampling an empty series and calling unique on that)
Can you have a look at that?

@jorisvandenbossche
Copy link
Member

@thanasis2028 Do you have time to update this PR?

@jreback
Copy link
Contributor

jreback commented Nov 3, 2016

can you rebase / update this?

@jreback
Copy link
Contributor

jreback commented Dec 16, 2016

this was closed by this PR here

@jreback jreback closed this Dec 16, 2016
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.

4 participants