Skip to content

ARROW-14844: [R] Implement decimal256() - #11898

Closed
dragosmg wants to merge 34 commits into
apache:masterfrom
dragosmg:ARROW-14844_decimal256_take2
Closed

ARROW-14844: [R] Implement decimal256()#11898
dragosmg wants to merge 34 commits into
apache:masterfrom
dragosmg:ARROW-14844_decimal256_take2

Conversation

@dragosmg

@dragosmgdragosmg commented Dec 8, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

@jonkeane & @romainfrancois this is the 2nd attempt at implementing decimal256(). First one is #11805

@github-actions

github-actionsBot commented Dec 8, 2021

Copy link
Copy Markdown

https://issues.apache.org/jira/browse/ARROW-14844

dragosmg
dragosmg commented Dec 8, 2021
View reviewed changes
Comment threadr/R/array.R Outdated
@dragosmgdragosmg mentioned this pull request Dec 8, 2021
Closed
@dragosmg
dragosmg marked this pull request as ready for review December 9, 2021 12:57
dragosmg
dragosmg commented Dec 9, 2021
View reviewed changes
Comment threadr/tests/testthat/test-chunked-array.R
@dragosmg

dragosmg commented Dec 9, 2021•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

It looks like the CI workflows fail for 2 reasons:

  • an open_dataset CSV test (BOM issue) which has nothing to do with this PR (see ARROW-15041)
  • failures to find and run /data-raw/codegen.R +
array_to_vector.cpp:923:62: error: typedef 'using DecimalArray = typename arrow::TypeTraits<T>::ArrayType' locally defined but not used [-Werror=unused-local-typedefs]

@romainfrancois@jonkeane Any thoughts on the array_to_vector error?

romainfrancois
romainfrancois reviewed Dec 10, 2021
View reviewed changes
Comment threadr/src/array_to_vector.cpp Outdated
romainfrancois
romainfrancois reviewed Dec 10, 2021
View reviewed changes
Comment threadr/src/array_to_vector.cpp Outdated
jonkeane
jonkeane reviewed Dec 13, 2021
View reviewed changes
Comment threadr/R/array.R
pitrou
pitrou reviewed Dec 15, 2021
View reviewed changes
Comment threadr/R/array.R Outdated
Comment threadr/src/array_to_vector.cpp
Comment threadr/tests/testthat/test-data-type.R
jonkeane
jonkeane reviewed Dec 20, 2021
View reviewed changes
Comment threadr/tests/testthat/test-Array.R Outdated
jonkeane
jonkeane reviewed Dec 20, 2021
View reviewed changes
Comment threadr/R/array.R
jonkeane
jonkeane reviewed Dec 20, 2021
View reviewed changes
Comment threadr/tests/testthat/test-Array.R Outdated
jonkeane
jonkeane approved these changes Dec 20, 2021
View reviewed changes

@jonkeanejonkeane left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks good. Thanks for this + all of the Jiras that you created in the process!

@jonkeanejonkeane changed the title ARROW-14844_decimal256_take2ARROW-14844: [R] Implement decimal256()Dec 20, 2021
@ursabot

ursabot commented Dec 20, 2021•
edited
Loading

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 281dee5 and contender = cfcce5a. cfcce5a is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️2.24% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.13% ⬆️0.04%] ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@dragosmgdragosmg mentioned this pull request Dec 20, 2021
Closed
@jonkeanejonkeane mentioned this pull request Dec 20, 2021
Closed
jonkeane added a commit that referenced this pull request Dec 20, 2021
@jonkeane
MINOR: [R] Clean up an overly-specific test
9ac9405
Follow on to #11898Closes#12005 from jonkeane/decimal-cleanup
Authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Jonathan Keane <jkeane@gmail.com>
@dragosmg
dragosmg deleted the ARROW-14844_decimal256_take2 branch February 24, 2022 09:25
This was referenced Dec 21, 2021
Closed
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pitroupitroupitrou left review comments
@romainfrancoisromainfrancoisromainfrancois left review comments
@jonkeanejonkeanejonkeane approved these changes

Assignees

No one assigned

Labels

Component: R

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dragosmg@jonkeane@ursabot@pitrou@romainfrancois