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 bindings for Fixed-point type support for Parquet #7153

Merged
merged 7 commits into from
Jan 21, 2021

Conversation

razajafri
Copy link
Contributor

@razajafri razajafri commented Jan 15, 2021

Adds in java support to be able to write fixed-point type to parquet

@razajafri razajafri added 3 - Ready for Review Ready for review by team Java Affects Java cuDF API. Spark Functionality that helps Spark RAPIDS 4 - Needs cuDF (Java) Reviewer improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jan 15, 2021
@razajafri razajafri requested review from a team as code owners January 15, 2021 05:43
@razajafri razajafri changed the title Java bindings for Fixed-point type support for Parquet [skip ci] Java bindings for Fixed-point type support for Parquet Jan 15, 2021
@razajafri
Copy link
Contributor Author

build

@razajafri
Copy link
Contributor Author

I have removed the [skip ci] as this change has some very small CPP header file change

java/src/main/native/src/TableJni.cpp Outdated Show resolved Hide resolved
java/src/main/native/src/TableJni.cpp Outdated Show resolved Hide resolved
@@ -846,6 +846,18 @@ class chunked_parquet_writer_options_builder {
return *this;
Copy link
Member

Choose a reason for hiding this comment

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

2021 copyrights should be added.

Copy link
Member

Choose a reason for hiding this comment

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

This is the only file cpp-codeowners need to approve. I will approve once the copyright is changed to 2020-2021

@@ -18,6 +18,10 @@

Copy link
Member

Choose a reason for hiding this comment

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

2021 copyrights

java/src/main/java/ai/rapids/cudf/Table.java Show resolved Hide resolved
java/src/main/native/src/TableJni.cpp Show resolved Hide resolved
Copy link
Contributor

@revans2 revans2 left a comment

Choose a reason for hiding this comment

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

I am OK with letting the current decimal precision API through if we feel that there are time constraints, but I really do find it ugly and decidedly not forward thinking.

@razajafri
Copy link
Contributor Author

build

@codecov
Copy link

codecov bot commented Jan 20, 2021

Codecov Report

Merging #7153 (143bf7a) into branch-0.18 (8860baf) will increase coverage by 0.07%.
The diff coverage is 73.33%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.18    #7153      +/-   ##
===============================================
+ Coverage        82.09%   82.17%   +0.07%     
===============================================
  Files               97       97              
  Lines            16474    16541      +67     
===============================================
+ Hits             13524    13592      +68     
+ Misses            2950     2949       -1     
Impacted Files Coverage Δ
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% <55.55%> (-1.61%) ⬇️
python/cudf/cudf/core/dtypes.py 89.50% <66.66%> (-0.88%) ⬇️
python/cudf/cudf/io/csv.py 91.66% <77.77%> (-1.67%) ⬇️
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% <100.00%> (-0.33%) ⬇️
python/cudf/cudf/core/column/string.py 86.65% <100.00%> (ø)
python/cudf/cudf/utils/cudautils.py 48.38% <0.00%> (-0.17%) ⬇️
... and 15 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 b0525f4...143bf7a. Read the comment docs.

@razajafri
Copy link
Contributor Author

build

@razajafri razajafri added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Jan 21, 2021
@revans2
Copy link
Contributor

revans2 commented Jan 21, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 4111cb7 into rapidsai:branch-0.18 Jan 21, 2021
@razajafri razajafri deleted the parquet_decimal branch January 21, 2021 17:36
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 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.

6 participants