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

Simplify preprocessing of arguments for DataFrame binops #10563

Merged
merged 9 commits into from
Apr 12, 2022

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Mar 31, 2022

This PR simplifies the preprocessing of the rhs of binary operations for DataFrame, streamlining it to a single path that can be more easily combined with that of other types in the future.

@vyasr vyasr added 3 - Ready for Review Ready for review by team Python Affects Python cuDF API. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change deprecation labels Mar 31, 2022
@vyasr vyasr added this to the CuDF Python Refactoring milestone Mar 31, 2022
@vyasr vyasr self-assigned this Mar 31, 2022
@vyasr vyasr requested a review from a team as a code owner March 31, 2022 21:30
@codecov
Copy link

codecov bot commented Mar 31, 2022

Codecov Report

Merging #10563 (c6de1c1) into branch-22.06 (73bc7d7) will increase coverage by 0.03%.
The diff coverage is n/a.

❗ Current head c6de1c1 differs from pull request most recent head 46fd129. Consider uploading reports for the commit 46fd129 to get more accurate results

@@               Coverage Diff                @@
##           branch-22.06   #10563      +/-   ##
================================================
+ Coverage         86.33%   86.37%   +0.03%     
================================================
  Files               140      140              
  Lines             22300    22304       +4     
================================================
+ Hits              19253    19265      +12     
+ Misses             3047     3039       -8     
Impacted Files Coverage Δ
python/cudf/cudf/testing/testing.py 81.69% <0.00%> (-2.82%) ⬇️
python/cudf/cudf/core/indexed_frame.py 91.77% <0.00%> (-0.87%) ⬇️
python/cudf/cudf/core/single_column_frame.py 96.52% <0.00%> (-0.33%) ⬇️
python/cudf/cudf/core/column/timedelta.py 90.75% <0.00%> (-0.12%) ⬇️
python/cudf/cudf/core/column/struct.py 96.42% <0.00%> (-0.09%) ⬇️
python/cudf/cudf/core/series.py 95.28% <0.00%> (-0.01%) ⬇️
python/cudf/cudf/datasets.py 97.56% <0.00%> (ø)
python/cudf/cudf/utils/ioutils.py 79.60% <0.00%> (ø)
python/cudf/cudf/utils/cudautils.py 59.83% <0.00%> (ø)
python/cudf/cudf/core/column/string.py 89.10% <0.00%> (ø)
... and 19 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 4775f11...46fd129. Read the comment docs.

Copy link
Contributor

@galipremsagar galipremsagar left a comment

Choose a reason for hiding this comment

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

Most of the PR looks good, I have some minor comments.

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/frame.py Show resolved Hide resolved
@vyasr vyasr force-pushed the refactor/binops_dataframe branch from bb6b309 to 46fd129 Compare April 12, 2022 20:49
@vyasr vyasr requested a review from galipremsagar April 12, 2022 20:50
@vyasr
Copy link
Contributor Author

vyasr commented Apr 12, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit c9e16c7 into rapidsai:branch-22.06 Apr 12, 2022
@vyasr vyasr deleted the refactor/binops_dataframe branch April 12, 2022 22:56
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.

2 participants