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

Introduce a common parent class for NumericalColumn and DecimalColumn #8278

Merged
merged 8 commits into from
May 26, 2021

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented May 18, 2021

Adds a common parent class for NumericalColumn and DecimalColumn that implements the common operations (mainly reductions and scans). As part of enabling this, the _copy_type_metadata method was refactored to have different column types override it to perform column-specific tasks (necessary because DecimalColumn has some special handling), which should also provide some marginal performance improvements for methods that copy metadata.

This PR (and some to follow) are designed to help simplify the ongoing Array refactoring.

@vyasr vyasr requested a review from a team as a code owner May 18, 2021 23:16
@vyasr vyasr requested review from kkraus14 and galipremsagar May 18, 2021 23:16
@vyasr vyasr self-assigned this May 18, 2021
@vyasr vyasr added the 3 - Ready for Review Ready for review by team label May 18, 2021
@github-actions github-actions bot added the Python Affects Python cuDF API. label May 18, 2021
@vyasr vyasr added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 18, 2021
@galipremsagar
Copy link
Contributor

@vyasr Looks like there are some test failures in test_transform.py that are linked to this refactor, could you take a look at it?

@galipremsagar galipremsagar added 0 - Waiting on Author Waiting for author to respond to review and removed 3 - Ready for Review Ready for review by team labels May 25, 2021
@vyasr
Copy link
Contributor Author

vyasr commented May 25, 2021

Yes sorry I noticed this failure but I've been focusing on #8214 for the past few days. I should hit a good stopping point this afternoon to come back and wrap up my open Python PRs including this one.

@vyasr vyasr added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 0 - Waiting on Author Waiting for author to respond to review labels May 25, 2021
@kkraus14 kkraus14 added breaking Breaking change and removed non-breaking Non-breaking change labels May 26, 2021
@kkraus14
Copy link
Collaborator

Marking this as breaking since we don't technically hide the Column class even though we don't intend for it to be part of our public API.

@codecov
Copy link

codecov bot commented May 26, 2021

Codecov Report

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

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.06    #8278   +/-   ##
===============================================
  Coverage                ?   82.88%           
===============================================
  Files                   ?      106           
  Lines                   ?    17844           
  Branches                ?        0           
===============================================
  Hits                    ?    14790           
  Misses                  ?     3054           
  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 cbbcba7...442fcda. Read the comment docs.

@kkraus14
Copy link
Collaborator

@gpucibot merge

@rapids-bot rapids-bot bot merged commit cfc7ef9 into rapidsai:branch-21.06 May 26, 2021
@vyasr vyasr added this to the cuDF Python Refactoring milestone Jul 22, 2021
@vyasr vyasr deleted the refactor/numerical_column_ops branch January 14, 2022 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge breaking Breaking change improvement Improvement / enhancement to an existing function Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants