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

Refactor array function #10364

Merged
merged 7 commits into from
Mar 2, 2022

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Feb 25, 2022

This PR cleans up the implementation of __array_function__ for Series and DataFrame to bring them further into alignment. It also inlines a number of functions defined in utils/utils.py that were previously used only in Series.__array_ufunc__, building on the improvements in #10217, #10287, and #10346 to clear out methods related to the old __array_ufunc__ dispatch that are now only used by this __array_function__ implementation. Inlining these methods also allows significant simplification since they were handling cases that are no longer relevant or possible. Unlike those previous PRs, this one does not actually enable any new features. Although it should marginally accelerate array functions by simplifying the dispatch logic, the fact that this API makes few promises about the nature of the function being applied and our desire to have it "just work" as much as possible means that we must simply adopt an EAFP approach and return NotImplemented if any part of the process fails.

@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 labels Feb 25, 2022
@vyasr vyasr added this to the CuDF Python Refactoring milestone Feb 25, 2022
@vyasr vyasr requested a review from a team as a code owner February 25, 2022 23:29
@vyasr vyasr self-assigned this Feb 25, 2022
@codecov
Copy link

codecov bot commented Feb 26, 2022

Codecov Report

Merging #10364 (d862b95) into branch-22.04 (a7d88cd) will increase coverage by 0.21%.
The diff coverage is n/a.

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

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-22.04   #10364      +/-   ##
================================================
+ Coverage         10.42%   10.64%   +0.21%     
================================================
  Files               119      126       +7     
  Lines             20603    20938     +335     
================================================
+ Hits               2148     2228      +80     
- Misses            18455    18710     +255     
Impacted Files Coverage Δ
...ython/custreamz/custreamz/tests/test_dataframes.py 99.39% <0.00%> (-0.01%) ⬇️
python/cudf/cudf/errors.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/ops.py 0.00% <0.00%> (ø)
python/cudf/cudf/datasets.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/index.py 0.00% <0.00%> (ø)
python/cudf/cudf/io/parquet.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/scalar.py 0.00% <0.00%> (ø)
... and 45 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 21325e8...ea5e88c. Read the comment docs.

python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
@vyasr vyasr requested a review from bdice February 28, 2022 23:51
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.

Beautiful!

python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
@vyasr
Copy link
Contributor Author

vyasr commented Mar 2, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit b4d262d into rapidsai:branch-22.04 Mar 2, 2022
@vyasr vyasr deleted the refactor/array_function branch March 9, 2022 17:21
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.

3 participants