Skip to content

Consolidate remaining DataFrame/Series APIs - #11315

Merged
rapids-bot[bot] merged 5 commits into
NVIDIA:branch-22.08from
vyasr:refactor/consolidate_frame_apis
Jul 26, 2022
Merged

Consolidate remaining DataFrame/Series APIs#11315
rapids-bot[bot] merged 5 commits into
NVIDIA:branch-22.08from
vyasr:refactor/consolidate_frame_apis

Conversation

@vyasr

@vyasr vyasr commented Jul 20, 2022

Copy link
Copy Markdown
Contributor

This PR resolves #9038. Not all the remaining functions could be consolidated (for various reasons), but the combination was explored and where necessary the reasons not to combine the functions was documented in that issue. Some additional cleanup that was done during the exploration is also included in this PR, but ultimately groupby is the only method that was moved up to IndexedFrame.

@vyasr vyasr added 3 - Ready for Review Ready for review by team code quality Python Affects Python cuDF API. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jul 20, 2022
@vyasr vyasr added this to the CuDF Python Refactoring milestone Jul 20, 2022
@vyasr vyasr self-assigned this Jul 20, 2022
@vyasr
vyasr requested a review from a team as a code owner July 20, 2022 22:59
@vyasr
vyasr requested review from bdice and skirui-source July 20, 2022 22:59
@vyasr

vyasr commented Jul 26, 2022

Copy link
Copy Markdown
Contributor Author

rerun tests

@codecov

codecov Bot commented Jul 26, 2022

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.08@9f57301). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-22.08   #11315   +/-   ##
===============================================
  Coverage                ?   86.40%           
===============================================
  Files                   ?      143           
  Lines                   ?    22756           
  Branches                ?        0           
===============================================
  Hits                    ?    19662           
  Misses                  ?     3094           
  Partials                ?        0           

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 9f57301...2d99643. Read the comment docs.

@vyasr

vyasr commented Jul 26, 2022

Copy link
Copy Markdown
Contributor Author

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit c07557c into NVIDIA:branch-22.08 Jul 26, 2022
@vyasr
vyasr deleted the refactor/consolidate_frame_apis branch July 26, 2022 21:51

@bdice bdice left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One quibble about dedent. Otherwise LGTM!



class GroupBy(Serializable, Reducible, Scannable):
groupby_doc_template = textwrap.dedent(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't the point of using dedent so that you can write it in a nicely indented way? (The catch is to escape the first newline iirc.)

groupby_doc_template = textwrap.dedent("""\
    Group using a mapper or by a Series of columns.

    A groupby operation involves...

    Parameters
    ----------
    by : ...
    """

@bdice

bdice commented Jul 26, 2022

Copy link
Copy Markdown
Contributor

@vyasr I was 3 minutes too slow!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for Review Ready for review by team 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] Consolidate shared method between DataFrame and Series into Frame

3 participants