Skip to content

Use new DecimalArray creation API in arrow crate#1249

Merged
alamb merged 9 commits intoapache:masterfrom
alamb:alamb/clean_up_decimal_creation2
Feb 15, 2022
Merged

Use new DecimalArray creation API in arrow crate#1249
alamb merged 9 commits intoapache:masterfrom
alamb:alamb/clean_up_decimal_creation2

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Jan 30, 2022

Builds on #1223 so draft until that is done

Rationale:

#1223 introduces a more performant and idiomatic API for creating DecimalArrays than DecimalBuilder so update the code to use that.

@github-actions github-actions bot added the arrow Changes to the arrow crate label Jan 30, 2022
@alamb alamb force-pushed the alamb/clean_up_decimal_creation2 branch from fc8ae0b to f64a3ea Compare January 30, 2022 12:46
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this re-validation occurs in DecimalBuilder as well, so this change isn't worse. I just noticed it while working on the code and figured I would leave a hint for future readers

@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2022

Codecov Report

Merging #1249 (11f1832) into master (35e16be) will decrease coverage by 0.01%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1249      +/-   ##
==========================================
- Coverage   83.04%   83.03%   -0.02%     
==========================================
  Files         180      180              
  Lines       52424    52365      -59     
==========================================
- Hits        43537    43479      -58     
+ Misses       8887     8886       -1     
Impacted Files Coverage Δ
arrow/src/compute/kernels/cast.rs 95.30% <86.66%> (+0.09%) ⬆️
arrow/src/compute/kernels/take.rs 95.27% <93.75%> (-0.08%) ⬇️
arrow/src/array/array_binary.rs 93.47% <100.00%> (-0.06%) ⬇️
arrow/src/array/equal/mod.rs 93.22% <100.00%> (-0.06%) ⬇️
arrow/src/array/equal_json.rs 89.70% <100.00%> (-0.21%) ⬇️
arrow/src/array/transform/mod.rs 84.52% <100.00%> (+<0.01%) ⬆️
arrow/src/compute/kernels/sort.rs 95.11% <100.00%> (-0.02%) ⬇️
arrow/src/ffi.rs 84.53% <100.00%> (-0.17%) ⬇️
arrow/src/util/pretty.rs 96.53% <100.00%> (-0.13%) ⬇️
arrow/src/datatypes/field.rs 53.79% <0.00%> (-0.31%) ⬇️
... and 3 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 35e16be...11f1832. Read the comment docs.

@alamb alamb force-pushed the alamb/clean_up_decimal_creation2 branch from f64a3ea to 11f1832 Compare February 8, 2022 20:14
@alamb alamb marked this pull request as ready for review February 8, 2022 20:14
@alamb
Copy link
Contributor Author

alamb commented Feb 8, 2022

FYI @liukun4515

@liukun4515
Copy link
Contributor

FYI @liukun4515

I will review this later in this weekend.

Copy link
Contributor

@liukun4515 liukun4515 left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks for your refactor. @alamb

@alamb
Copy link
Contributor Author

alamb commented Feb 15, 2022

Thanks @liukun4515

@alamb alamb merged commit 747e72a into apache:master Feb 15, 2022
@alamb alamb deleted the alamb/clean_up_decimal_creation2 branch February 15, 2022 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants