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

Remove various unused functions #9922

Merged
merged 12 commits into from
Jan 3, 2022

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Dec 16, 2021

This PR removes a number of unused functions and inlines some helpers that are only called in one place. This PR also deprecates Series.fill, which does not appear to be a pandas API. This PR resolves #9824.

@vyasr vyasr added 3 - Ready for Review Ready for review by team Python Affects Python cuDF API. tech debt improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Dec 16, 2021
@vyasr vyasr added this to the CuDF Python Refactoring milestone Dec 16, 2021
@vyasr vyasr self-assigned this Dec 16, 2021
@vyasr vyasr requested a review from a team as a code owner December 16, 2021 17:59
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

I'm reviewing this to help resolve #9824. I see a few inconsistencies with imports, otherwise LGTM.

python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/dataframe.py Show resolved Hide resolved
python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved
@vyasr vyasr requested a review from bdice December 16, 2021 22:18
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

LGTM!

@bdice
Copy link
Contributor

bdice commented Dec 20, 2021

@vyasr CI is failing for dask-cudf because the removed function _get_numeric_data is called by dask in two places:

@vyasr
Copy link
Contributor Author

vyasr commented Dec 21, 2021

  • numeric

I saw, just hadn't gotten around to fixing it yet. Will address it later today. Those specific calls aren't the issue, because they're calls to methods of the dask DataFrame objects and dask.dataframe.DataFrame implements the method. The problem is that dask.dataframe.DataFrame.describe assumes that the underlying DataFrame library (in this case, cudf.DataFrame) has the method. So I think we're stuck just leaving the method there as is.

@vyasr
Copy link
Contributor Author

vyasr commented Dec 22, 2021

rerun tests

@bdice
Copy link
Contributor

bdice commented Dec 23, 2021

I merged in branch-22.02 with my fix from #9951 to help make CI pass. This should be mergeable once CI passes.

@bdice bdice added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Dec 23, 2021
@codecov
Copy link

codecov bot commented Dec 23, 2021

Codecov Report

Merging #9922 (ea6b3d3) into branch-22.02 (04f4219) will increase coverage by 0.04%.
The diff coverage is 0.00%.

❗ Current head ea6b3d3 differs from pull request most recent head 3637574. Consider uploading reports for the commit 3637574 to get more accurate results
Impacted file tree graph

@@               Coverage Diff                @@
##           branch-22.02    #9922      +/-   ##
================================================
+ Coverage         10.37%   10.41%   +0.04%     
================================================
  Files               119      119              
  Lines             20111    20481     +370     
================================================
+ Hits               2086     2134      +48     
- Misses            18025    18347     +322     
Impacted Files Coverage Δ
python/cudf/cudf/core/dataframe.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/series.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/hdf.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/orc.py 0.00% <0.00%> (ø)
python/cudf/cudf/_version.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/abc.py 0.00% <0.00%> (ø)
python/cudf/cudf/api/types.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/dlpack.py 0.00% <0.00%> (ø)
... and 51 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 04f4219...3637574. Read the comment docs.

@vyasr
Copy link
Contributor Author

vyasr commented Jan 3, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 7233765 into rapidsai:branch-22.02 Jan 3, 2022
@vyasr vyasr deleted the refactor/misc_cleanup branch January 14, 2022 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Remove DataFrame._repr_pandas025_formatting.
3 participants