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

Warn when downscaling decimal columns #8492

Merged
merged 4 commits into from
Jun 18, 2021
Merged

Warn when downscaling decimal columns #8492

merged 4 commits into from
Jun 18, 2021

Conversation

ChrisJar
Copy link
Contributor

Closes #8484

@ChrisJar ChrisJar requested a review from a team as a code owner June 10, 2021 23:21
@github-actions github-actions bot added the Python Affects Python cuDF API. label Jun 10, 2021
@codecov
Copy link

codecov bot commented Jun 11, 2021 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.08    #8492   +/-   ##
===============================================
  Coverage                ?   82.95%           
===============================================
  Files                   ?      110           
  Lines                   ?    18184           
  Branches                ?        0           
===============================================
  Hits                    ?    15085           
  Misses                  ?     3099           
  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 1314bd5...37b3723. Read the comment docs.

shwina
shwina reviewed Jun 15, 2021
View reviewed changes
python/cudf/cudf/core/column/decimal.py Outdated
@@ -146,6 +147,16 @@ def _decimal_quantile(
def as_decimal_column(
self, dtype: Dtype, **kwargs
) -> "cudf.core.column.DecimalColumn":
if (
isinstance(dtype, Decimal64Dtype)
and isinstance(self.dtype, Decimal64Dtype)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we avoid this check, given. self is a DecimalColumn?

@ChrisJar ChrisJar requested a review from shwina June 15, 2021 21:58
@shwina
Copy link
Contributor

shwina commented Jun 15, 2021

@gpucibot merge

@charlesbluca
Copy link
Member

Any reason this never got merged?

@shwina
Copy link
Contributor

shwina commented Jun 18, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 90fe10b into rapidsai:branch-21.08 Jun 18, 2021
@vyasr vyasr added 4 - Needs Review Waiting for reviewer to review or respond and removed 4 - Needs cuDF (Python) Reviewer labels Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charlesbluca charlesbluca charlesbluca approved these changes

@shwina shwina shwina approved these changes

@skirui-source skirui-source Awaiting requested review from skirui-source

Assignees
No one assigned
Labels
4 - Needs Review Waiting for reviewer to review or respond 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] Add WARN on truncate vs round behavior when calling astype on Decimal columns
4 participants
@ChrisJar @shwina @charlesbluca @vyasr