Skip to content

Cleanup of GroupBy Code #26090

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

Merged
merged 5 commits into from
Apr 16, 2019
Merged

Cleanup of GroupBy Code #26090

merged 5 commits into from
Apr 16, 2019

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Apr 14, 2019

Removing some dead code I came across on review of the module which appears in NDFrameGroupBy but is overridden by its only public child class DataFrameGroupBy

FWIW with the removal of PanelGroupBy the NDFrameGroupBy is an unnecessary layer of inheritance and could probably be merged in with DataFrameGroupBy, though I figure that could be a separate dedicated PR to merge those if so desired

@WillAyd WillAyd changed the title Groupby cleanup Cleanup of GroupBy Code Apr 14, 2019
@codecov
Copy link

codecov bot commented Apr 15, 2019

Codecov Report

Merging #26090 into master will decrease coverage by 51.19%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26090      +/-   ##
==========================================
- Coverage   91.96%   40.76%   -51.2%     
==========================================
  Files         175      175              
  Lines       52412    52419       +7     
==========================================
- Hits        48200    21369   -26831     
- Misses       4212    31050   +26838
Flag Coverage Δ
#multiple ?
#single 40.76% <ø> (-0.11%) ⬇️
Impacted Files Coverage Δ
pandas/core/groupby/groupby.py 24.52% <ø> (-72.71%) ⬇️
pandas/core/groupby/generic.py 13.26% <ø> (-73.83%) ⬇️
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/core/groupby/categorical.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
pandas/io/formats/html.py 0% <0%> (-99.37%) ⬇️
pandas/io/sas/sas7bdat.py 0% <0%> (-91.16%) ⬇️
pandas/io/sas/sas_xport.py 0% <0%> (-90.1%) ⬇️
... and 135 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 7b3bf2d...89bd626. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 15, 2019

Codecov Report

Merging #26090 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26090      +/-   ##
==========================================
+ Coverage   91.96%   91.98%   +0.02%     
==========================================
  Files         175      175              
  Lines       52412    52392      -20     
==========================================
- Hits        48200    48193       -7     
+ Misses       4212     4199      -13
Flag Coverage Δ
#multiple 90.54% <ø> (+0.02%) ⬆️
#single 40.74% <ø> (-0.13%) ⬇️
Impacted Files Coverage Δ
pandas/core/groupby/generic.py 89.03% <ø> (+1.95%) ⬆️
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/frame.py 96.9% <0%> (-0.12%) ⬇️

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 7b3bf2d...89bd626. Read the comment docs.

@WillAyd
Copy link
Member Author

WillAyd commented Apr 15, 2019

Looks like the _assure_grouper method is actually subclassed by DateTimeIndexResample so added that back in for now

@jreback jreback added this to the 0.25.0 milestone Apr 16, 2019
@jreback jreback merged commit 32d66e0 into pandas-dev:master Apr 16, 2019
@jreback
Copy link
Contributor

jreback commented Apr 16, 2019

thanks!

yhaque1213 pushed a commit to yhaque1213/pandas that referenced this pull request Apr 22, 2019
@WillAyd WillAyd deleted the groupby-cleanup branch January 16, 2020 00:34
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.

3 participants