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

Java support of casting string from/to decimal [skip ci] #7623

Merged
merged 2 commits into from
Mar 23, 2021

Conversation

sperlingxx
Copy link
Contributor

This pull request provided Java side support of casting string from/to decimal, which is required by spark-rapids.

Although parsing of string formatted as scientific notation to decimal has not been supported yet by cuDF, we are able to implement string to decimal conversion at spark-rapids side through a two-steps hack:

  1. casting string to float
  2. casting float to decimal

In addition, this pull request also addressed issue #6795.

@sperlingxx sperlingxx requested a review from a team as a code owner March 17, 2021 08:25
@sperlingxx sperlingxx changed the title Java support of casting string from/to decimal Java support of casting string from/to decimal [skip-ci] Mar 17, 2021
@github-actions github-actions bot added the Java Affects Java cuDF API. label Mar 17, 2021
@sperlingxx sperlingxx added 4 - Needs cuDF (Java) Reviewer 5 - Ready to Merge Testing and reviews complete, ready to merge improvement Improvement / enhancement to an existing function Spark Functionality that helps Spark RAPIDS non-breaking Non-breaking change and removed 5 - Ready to Merge Testing and reviews complete, ready to merge labels Mar 17, 2021
@sperlingxx sperlingxx changed the title Java support of casting string from/to decimal [skip-ci] Java support of casting string from/to decimal [skip ci] Mar 17, 2021

/**
* Helper function to create decimal strings which can be processed by castStringToDecimal functor.
* We can not simply create decimal string via `String.valueOf`, because castStringToDecimal doesn't
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please file a follow on issue for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've filed issue for scientific notation. And appended the issue link here.

@codecov
Copy link

codecov bot commented Mar 17, 2021

Codecov Report

Merging #7623 (95f3313) into branch-0.19 (7871e7a) will increase coverage by 0.54%.
The diff coverage is 93.75%.

❗ Current head 95f3313 differs from pull request most recent head 49e5043. Consider uploading reports for the commit 49e5043 to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.19    #7623      +/-   ##
===============================================
+ Coverage        81.86%   82.41%   +0.54%     
===============================================
  Files              101      101              
  Lines            16884    17353     +469     
===============================================
+ Hits             13822    14301     +479     
+ Misses            3062     3052      -10     
Impacted Files Coverage Δ
python/cudf/cudf/core/index.py 93.34% <ø> (+0.48%) ⬆️
python/cudf/cudf/core/column/numerical.py 94.83% <87.50%> (-0.20%) ⬇️
python/cudf/cudf/core/frame.py 89.09% <89.47%> (+0.08%) ⬆️
python/cudf/cudf/core/column/column.py 87.86% <90.00%> (+0.10%) ⬆️
python/cudf/cudf/core/column/decimal.py 92.75% <90.32%> (-2.12%) ⬇️
python/cudf/cudf/core/dataframe.py 90.58% <95.65%> (+0.11%) ⬆️
python/cudf/cudf/core/series.py 91.57% <95.83%> (+0.79%) ⬆️
python/cudf/cudf/core/column/categorical.py 91.97% <100.00%> (+0.58%) ⬆️
python/cudf/cudf/core/column/datetime.py 89.63% <100.00%> (+0.54%) ⬆️
python/cudf/cudf/core/column/string.py 86.76% <100.00%> (+0.26%) ⬆️
... and 55 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 2488bc8...49e5043. Read the comment docs.

@sperlingxx
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 4e9241e into rapidsai:branch-0.19 Mar 23, 2021
@sperlingxx sperlingxx deleted the str_to_fixed_point branch March 23, 2021 00:26
@vyasr vyasr added 4 - Needs Review Waiting for reviewer to review or respond and removed 4 - Needs cuDF (Java) Reviewer labels Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - Needs Review Waiting for reviewer to review or respond improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. non-breaking Non-breaking change Spark Functionality that helps Spark RAPIDS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants