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.

Sign up for GitHub

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

Jump to bottom

Enable round in cudf for DataFrame and Series #7022

Merged
merged 21 commits into from
Jan 20, 2021
Merged

Enable round in cudf for DataFrame and Series #7022

merged 21 commits into from
Jan 20, 2021

Conversation

ChrisJar
Copy link
Contributor

@ChrisJar ChrisJar commented Dec 16, 2020 •
edited
Loading

This enables round for DataFrames and Series using the libcudf round implementation and removes the old numba round implementation.

Closes #1270

@codecov
Copy link

codecov bot commented Dec 16, 2020 •
edited
Loading

Codecov Report

Merging #7022 (d672059) into branch-0.18 (8860baf) will increase coverage by 0.07%.
The diff coverage is 85.12%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.18    #7022      +/-   ##
===============================================
+ Coverage        82.09%   82.16%   +0.07%     
===============================================
  Files               97       97              
  Lines            16474    16535      +61     
===============================================
+ Hits             13524    13586      +62     
+ Misses            2950     2949       -1
Impacted Files Coverage Δ
python/cudf/cudf/_lib/__init__.py 100.00% <ø> (ø)
python/cudf/cudf/core/column/lists.py 91.66% <ø> (-0.09%) ⬇️
python/cudf/cudf/core/dataframe.py 90.71% <50.00%> (+<0.01%) ⬆️
python/cudf/cudf/io/orc.py 86.80% <60.00%> (-1.61%) ⬇️
python/cudf/cudf/core/dtypes.py 89.50% <66.66%> (-0.88%) ⬇️
python/cudf/cudf/io/csv.py 90.74% <77.77%> (-2.60%) ⬇️
python/cudf/cudf/core/series.py 91.10% <80.00%> (-0.06%) ⬇️
python/cudf/cudf/core/reshape.py 91.00% <81.81%> (-0.04%) ⬇️
python/cudf/cudf/core/column/numerical.py 94.08% <83.33%> (-0.33%) ⬇️
python/cudf/cudf/core/frame.py 89.90% <88.88%> (-0.08%) ⬇️
... and 10 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 02e25b6...d672059. Read the comment docs.

@ChrisJar ChrisJar marked this pull request as ready for review December 17, 2020 22:40
@ChrisJar ChrisJar requested a review from a team as a code owner December 17, 2020 22:40
@shwina
Copy link
Contributor

shwina commented Dec 18, 2020

Looks very thorough - great work! Just a suggestion for improving the implementation for DataFrames.

@ChrisJar
Copy link
Contributor Author

Looks very thorough - great work! Just a suggestion for improving the implementation for DataFrames.

Thanks for the feedback! I'll make those changes shortly

@rgsl888prabhu rgsl888prabhu added 4 - Needs cuDF (Python) Reviewer Python Affects Python cuDF API. feature request New feature or request non-breaking Non-breaking change labels Dec 18, 2020
rgsl888prabhu
rgsl888prabhu reviewed Dec 18, 2020
View reviewed changes
Copy link
Contributor

@rgsl888prabhu rgsl888prabhu left a comment

Choose a reason for hiding this comment

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

small comments and questions. Otherwise looks pretty good.

python/cudf/cudf/_lib/round.pyx Outdated Show resolved Hide resolved
python/cudf/cudf/core/frame.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/frame.py Outdated Show resolved Hide resolved
python/cudf/cudf/tests/test_dataframe.py Outdated Show resolved Hide resolved
python/cudf/cudf/tests/test_dataframe.py Outdated Show resolved Hide resolved
rgsl888prabhu
rgsl888prabhu reviewed Jan 11, 2021
View reviewed changes
Copy link
Contributor

@rgsl888prabhu rgsl888prabhu left a comment

Choose a reason for hiding this comment

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

LGTM, couple of small questions.

python/cudf/cudf/_lib/round.pyx Outdated Show resolved Hide resolved
python/cudf/cudf/core/frame.py Outdated Show resolved Hide resolved
isVoid
isVoid requested changes Jan 11, 2021
View reviewed changes
python/cudf/cudf/core/frame.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/frame.py Outdated Show resolved Hide resolved
python/cudf/cudf/tests/test_dataframe.py Show resolved Hide resolved
isVoid
isVoid approved these changes Jan 11, 2021
View reviewed changes
Copy link
Contributor

@isVoid isVoid left a comment

Choose a reason for hiding this comment

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

LGTM! Awesome work!

@galipremsagar galipremsagar added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 4 - Needs cuDF (Python) Reviewer labels Jan 20, 2021
@galipremsagar
Copy link
Contributor

@gpucibot merge

@rapids-bot rapids-bot bot merged commit a51caa5 into rapidsai:branch-0.18 Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgsl888prabhu rgsl888prabhu rgsl888prabhu approved these changes

@isVoid isVoid isVoid approved these changes

@galipremsagar galipremsagar galipremsagar approved these changes

@shwina shwina shwina approved these changes

Assignees
No one assigned
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge feature request New feature or request non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] Expose libcudf column round in Python
5 participants
@ChrisJar @shwina @galipremsagar @isVoid @rgsl888prabhu