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

BUG: Handle NA values for ExtensionArrays in Series.count #26836

Merged
merged 4 commits into from Jun 21, 2019
Merged

BUG: Handle NA values for ExtensionArrays in Series.count #26836

merged 4 commits into from Jun 21, 2019

Conversation

ghost
Copy link

@ghost ghost commented Jun 13, 2019

@pep8speaks
Copy link

pep8speaks commented Jun 13, 2019

Hello @pilkibun! 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 2019-06-18 00:59:02 UTC

@ghost ghost changed the title Series count ea support @pilkibun BUG: Handle NA values for ExtensionArrays in Series.count Jun 13, 2019
@ghost ghost changed the title @pilkibun BUG: Handle NA values for ExtensionArrays in Series.count BUG: Handle NA values for ExtensionArrays in Series.count Jun 13, 2019
doc/source/whatsnew/v0.25.0.rst Outdated Show resolved Hide resolved
doc/source/whatsnew/v0.25.0.rst Outdated Show resolved Hide resolved
pandas/tests/extension/decimal/test_decimal.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jun 13, 2019

Codecov Report

Merging #26836 into master will increase coverage by 1.4%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #26836     +/-   ##
=========================================
+ Coverage   90.45%   91.86%   +1.4%     
=========================================
  Files         179      179             
  Lines       50706    50706             
=========================================
+ Hits        45866    46579    +713     
+ Misses       4840     4127    -713
Flag Coverage Δ
#multiple 90.45% <100%> (ø) ⬆️
#single 41.1% <100%> (?)
Impacted Files Coverage Δ
pandas/core/series.py 93.64% <100%> (ø) ⬆️
pandas/core/arrays/categorical.py 95.92% <0%> (+0.12%) ⬆️
pandas/core/indexes/datetimes.py 96.37% <0%> (+0.16%) ⬆️
pandas/core/dtypes/missing.py 93.93% <0%> (+0.6%) ⬆️
pandas/io/formats/printing.py 85.56% <0%> (+1.06%) ⬆️
pandas/io/clipboard/clipboards.py 34.78% <0%> (+2.89%) ⬆️
pandas/core/computation/expr.py 97.8% <0%> (+3.02%) ⬆️
pandas/core/computation/common.py 89.47% <0%> (+5.26%) ⬆️
pandas/io/pytables.py 90.29% <0%> (+26.47%) ⬆️
pandas/core/computation/pytables.py 90.24% <0%> (+27.74%) ⬆️

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 e8132a2...c54b3f6. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 13, 2019

Codecov Report

Merging #26836 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26836      +/-   ##
==========================================
- Coverage   91.86%   91.86%   -0.01%     
==========================================
  Files         179      180       +1     
  Lines       50700    50712      +12     
==========================================
+ Hits        46576    46586      +10     
- Misses       4124     4126       +2
Flag Coverage Δ
#multiple 90.45% <100%> (ø) ⬆️
#single 41.1% <100%> (-0.08%) ⬇️
Impacted Files Coverage Δ
pandas/core/series.py 93.64% <100%> (ø) ⬆️
pandas/io/gbq.py 88.88% <0%> (-11.12%) ⬇️
pandas/io/formats/printing.py 85.56% <0%> (-0.54%) ⬇️
pandas/core/frame.py 96.88% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.84% <0%> (-0.11%) ⬇️
pandas/core/arrays/datetimelike.py 97.92% <0%> (-0.01%) ⬇️
pandas/core/internals/managers.py 95.21% <0%> (-0.01%) ⬇️
pandas/core/internals/blocks.py 94.38% <0%> (-0.01%) ⬇️
pandas/core/arrays/datetimes.py 97.79% <0%> (-0.01%) ⬇️
pandas/core/dtypes/base.py 100% <0%> (ø) ⬆️
... and 22 more

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 2d2606d...2bbb256. Read the comment docs.

@jreback jreback added Bug ExtensionArray Extending pandas with custom dtypes or arrays. labels Jun 13, 2019
@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jun 13, 2019 via email

@ghost
Copy link
Author

ghost commented Jun 13, 2019

yeah. more generally It shows there might be lots of bugs still present when frame methods should but don't return the same result as column by column application, due to EA changes.

@ghost
Copy link
Author

ghost commented Jun 13, 2019

btw, what's the norm here, force-push or add commits after review?

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jun 13, 2019 via email

@ghost
Copy link
Author

ghost commented Jun 13, 2019

tests pass and I think I addressed all comments. Ready to merge?

doc/source/whatsnew/v0.25.0.rst Outdated Show resolved Hide resolved
@TomAugspurger
Copy link
Contributor

@pilkibun can you make an ExtensionArray section, and move your release note there?

@ghost
Copy link
Author

ghost commented Jun 18, 2019

added

@TomAugspurger TomAugspurger merged commit 9aef32d into pandas-dev:master Jun 21, 2019
@TomAugspurger
Copy link
Contributor

Thanks!

@ghost ghost deleted the series_count_EA_support branch June 22, 2019 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug ExtensionArray Extending pandas with custom dtypes or arrays.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Series.Count() miscounts NA values in ExtensionArrays
4 participants