Skip to content

Ensure the right values are set in SeriesGroupBy.nunique #15418

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 2 commits into from

Conversation

aiguofer
Copy link
Contributor

We only need to use the group boundaries as the index for res so that
the dimensions match those of out. Fixes #13453

We only need to use the group boundaries as the index for `res` so that
the dimensions match those of `out`. Fixes pandas-dev#13453
@jreback
Copy link
Contributor

jreback commented Feb 15, 2017

can you add all 4 comparisions (e.g. .count == .nunique, #13453 (comment)) and using .agg #13453 (comment)

so you have 4 cases that you are comparing for equality (you can call them result1-4 and compare)

@jreback
Copy link
Contributor

jreback commented Feb 15, 2017

pls also add a bug fix note in 0.20.0. lgtm otherwise. ping on green! (with test additions)

@jreback
Copy link
Contributor

jreback commented Feb 15, 2017

@aiguofer you shouldn't have any tests fail. Note we have recently switched to pytest.

@aiguofer
Copy link
Contributor Author

Added test to resample since it made more sense there... I also added .agg('nunique') for completeness.

When I run tests with pytest all passes :)

@codecov-io
Copy link

Codecov Report

Merging #15418 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #15418   +/-   ##
=======================================
  Coverage   90.36%   90.36%           
=======================================
  Files         135      135           
  Lines       49438    49438           
=======================================
  Hits        44675    44675           
  Misses       4763     4763
Impacted Files Coverage Δ
pandas/core/groupby.py 94.89% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6f8b46...c53bd70. Read the comment docs.

@aiguofer
Copy link
Contributor Author

@jreback all looks good :)

@jreback jreback closed this in 5a8883b Feb 16, 2017
@jreback jreback added this to the 0.20.0 milestone Feb 16, 2017
@jreback
Copy link
Contributor

jreback commented Feb 16, 2017

thanks @aiguofer

keep em coming!

jreback added a commit to jreback/pandas that referenced this pull request Feb 17, 2017
jreback added a commit to jreback/pandas that referenced this pull request Feb 18, 2017
jreback added a commit that referenced this pull request Feb 18, 2017
- we are passing builds which actually have an error
- fix the small dtype issues

Author: Jeff Reback <[email protected]>

Closes #15445 from jreback/windows and squashes the following commits:

a5b7fb3 [Jeff Reback] change integer to power comparisions
eab15c4 [Jeff Reback] don't force remove pandas
cf3b9bd [Jeff Reback] more windows fixing
efe6a76 [Jeff Reback] add cython to build
8194e63 [Jeff Reback] don't use appveyor recipe, just build inplace
e064825 [Jeff Reback] TST: resample dtype issue xref #15418
10d9b26 [Jeff Reback] TST: run windows tests so failures show up in appeveyor
AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request Mar 21, 2017
closes pandas-dev#13453

Author: Diego Fernandez <[email protected]>

Closes pandas-dev#15418 from aiguofer/gh_13453 and squashes the following commits:

c53bd70 [Diego Fernandez] Add test for pandas-dev#13453 in test_resample and add note to whatsnew
0daab80 [Diego Fernandez] Ensure the right values are set in SeriesGroupBy.nunique
AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request Mar 21, 2017
- we are passing builds which actually have an error
- fix the small dtype issues

Author: Jeff Reback <[email protected]>

Closes pandas-dev#15445 from jreback/windows and squashes the following commits:

a5b7fb3 [Jeff Reback] change integer to power comparisions
eab15c4 [Jeff Reback] don't force remove pandas
cf3b9bd [Jeff Reback] more windows fixing
efe6a76 [Jeff Reback] add cython to build
8194e63 [Jeff Reback] don't use appveyor recipe, just build inplace
e064825 [Jeff Reback] TST: resample dtype issue xref pandas-dev#15418
10d9b26 [Jeff Reback] TST: run windows tests so failures show up in appeveyor
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.

Resampler.nunique counting data more than once
3 participants