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

Use generator expressions in any/all functions. #10736

Merged
merged 11 commits into from
Apr 30, 2022

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Apr 26, 2022

This PR uses generator expressions in any(...) and all(...) to avoid allocating a list in memory while maximizing the potential benefit of early exit from the any/all function.

I also fixed a few miscellaneous things (~ 10 lines):

  • Use cls in classmethods
  • Simplify a lambda expression
  • Use super() with no arguments if the arguments are the parent class and self
  • Parenthesize multi-line strings with implicit concatenation to clarify the behavior when written in a tuple of values

Note: Some of these were caught by https://codereview.doctor/rapidsai/cudf. In some places, the bot correctly identified a problem but its suggestions were invalid or incomplete. I identified steps for improvement beyond what the bot suggested for most of these cases.

@bdice bdice added code quality Python Affects Python cuDF API. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 26, 2022
@bdice bdice self-assigned this Apr 26, 2022
@bdice bdice requested a review from a team as a code owner April 26, 2022 14:39
@bdice bdice requested a review from isVoid April 26, 2022 14:39
@bdice bdice requested a review from charlesbluca April 26, 2022 14:39
@bdice
Copy link
Contributor Author

bdice commented Apr 26, 2022

rerun tests

1 similar comment
@bdice
Copy link
Contributor Author

bdice commented Apr 28, 2022

rerun tests

@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

Merging #10736 (75f1769) into branch-22.06 (84f88ce) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 75f1769 differs from pull request most recent head 9b0fd6b. Consider uploading reports for the commit 9b0fd6b to get more accurate results

@@               Coverage Diff                @@
##           branch-22.06   #10736      +/-   ##
================================================
+ Coverage         86.40%   86.43%   +0.02%     
================================================
  Files               143      143              
  Lines             22444    22444              
================================================
+ Hits              19393    19399       +6     
+ Misses             3051     3045       -6     
Impacted Files Coverage Δ
python/cudf/cudf/_fuzz_testing/utils.py 0.00% <ø> (ø)
python/cudf/cudf/comm/gpuarrow.py 79.76% <ø> (ø)
python/cudf/cudf/core/column/string.py 89.21% <ø> (+0.12%) ⬆️
python/cudf/cudf/core/frame.py 93.41% <ø> (ø)
python/cudf/cudf/core/series.py 95.16% <ø> (ø)
python/cudf/cudf/core/column/interval.py 83.67% <100.00%> (ø)
python/cudf/cudf/core/column/struct.py 96.42% <100.00%> (ø)
python/cudf/cudf/core/dataframe.py 93.74% <100.00%> (+0.04%) ⬆️
python/cudf/cudf/core/tools/datetimes.py 84.49% <100.00%> (+0.30%) ⬆️
python/cudf/cudf/io/parquet.py 92.70% <100.00%> (ø)
... and 6 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 84f88ce...9b0fd6b. Read the comment docs.

@bdice
Copy link
Contributor Author

bdice commented Apr 30, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 027c34a into rapidsai:branch-22.06 Apr 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants