Skip to content

ARROW-3815 [R]: refine record batch factory - #4508

Closed
romainfrancois wants to merge 5 commits into
apache:masterfrom
romainfrancois:ARROW-3815/record_batch
Closed

ARROW-3815 [R]: refine record batch factory#4508
romainfrancois wants to merge 5 commits into
apache:masterfrom
romainfrancois:ARROW-3815/record_batch

Conversation

@romainfrancois

@romainfrancoisromainfrancois commented Jun 10, 2019

Copy link
Copy Markdown
Contributor

so that array() when given an arrow::Array is identity

library(arrow, warn.conflicts=FALSE)
array(array(1:10))
#> arrow::Array #> [#> 1,#> 2,#> 3,#> 4,#> 5,#> 6,#> 7,#> 8,#> 9,#> 10#> ]

so that we may supply already made arrow::Array in record_batch()

batch<- record_batch(x=1:10, y=arrow::array(1:10))
as_tibble(batch)
#> # A tibble: 10 x 2#> x y#> <int> <int>#> 1 1 1#> 2 2 2#> 3 3 3#> 4 4 4#> 5 5 5#> 6 6 6#> 7 7 7#> 8 8 8#> 9 9 9#> 10 10 10

Created on 2019-06-10 by the reprex package (v0.3.0.9000)

@nealrichardson

Copy link
Copy Markdown
Member

Looks like cpp linting failed (wonder if we can make a precommit hook to fix style issues for us).

This will conflict with #4491

Do any docs need to be updated to explain valid inputs for these functions?

@romainfrancois

Copy link
Copy Markdown
ContributorAuthor

Do you have an example pre-commit script in mind ?

@codecov-io

codecov-io commented Jun 11, 2019

Copy link
Copy Markdown

Codecov Report

Merging #4508 into master will decrease coverage by 0.05%.
The diff coverage is 84.61%.

Impacted file tree graph

@@ Coverage Diff @@## master #4508 +/- ##
==========================================
- Coverage 88.15% 88.1% -0.06% 
==========================================
Files 850 850 Lines 106319 105783 -536 Branches 1253 1253 ==========================================
- Hits 93730 93204 -526 + Misses 12344 12334 -10 
Partials 245 245
Impacted FilesCoverage Δ
r/src/arrowExports.cpp72.7% <ø> (ø)⬆️
r/src/array_from_vector.cpp78.36% <83.33%> (+0.06%)⬆️
r/src/recordbatch.cpp82.69% <85.71%> (ø)⬆️
r/src/arrow_types.h88.88% <0%> (-7.12%)⬇️
cpp/src/gandiva/decimal_ir.cc72.78% <0%> (-3.82%)⬇️
cpp/src/arrow/json/converter.cc90.53% <0%> (-1.78%)⬇️
cpp/src/arrow/json/chunked-builder.cc79.91% <0%> (-1.68%)⬇️
r/src/array__to_vector.cpp77.95% <0%> (-0.71%)⬇️
cpp/src/gandiva/precompiled/decimal_ops.cc96.68% <0%> (-0.51%)⬇️
cpp/src/gandiva/function_registry_arithmetic.cc100% <0%> (ø)⬆️
... and 11 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 365ec5b...189ab97. Read the comment docs.

@nealrichardson

Copy link
Copy Markdown
Member

Re: precommit hook, IDK exactly, I've just seen lint failures on Travis where the error message tells you a script to run to fix it, so why not just run that script in a precommit hook and prevent the Travis failure in the first place? If this is sufficiently annoying to you too, I can look into it.

@romainfrancois
romainfrancoisforce-pushed the ARROW-3815/record_batch branch from c3f00b7 to feef4fdCompareJune 12, 2019 08:01
@romainfrancois
romainfrancoisforce-pushed the ARROW-3815/record_batch branch from feef4fd to 189ab97CompareJune 12, 2019 10:46
@romainfrancois
romainfrancois deleted the ARROW-3815/record_batch branch June 12, 2019 11:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@romainfrancois@nealrichardson@codecov-io