From 2925a7400eb8202f7ed48fb45c707f5f87c02b40 Mon Sep 17 00:00:00 2001 From: Romain Francois Date: Mon, 10 Jun 2019 14:53:19 +0200 Subject: [PATCH 1/5] InferType recognizes arrow::Array --- r/src/array_from_vector.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/r/src/array_from_vector.cpp b/r/src/array_from_vector.cpp index 0c21e94e277..6c5d0533e5b 100644 --- a/r/src/array_from_vector.cpp +++ b/r/src/array_from_vector.cpp @@ -763,6 +763,12 @@ std::shared_ptr GetFactorType(SEXP factor) { std::shared_ptr InferType(SEXP x) { switch (TYPEOF(x)) { + case ENVSXP: + if (Rf_inherits(x, "arrow::Array")) { + Rcpp::ConstReferenceSmartPtrInputParameter> array(x); + return static_cast>(array)->type(); + } + break; case LGLSXP: return boolean(); case INTSXP: From f17ae03186876cf25200fba44b3991fd1b1d9fb3 Mon Sep 17 00:00:00 2001 From: Romain Francois Date: Mon, 10 Jun 2019 15:01:49 +0200 Subject: [PATCH 2/5] Array__from_vector() recosgnise arrow::Array --- r/src/array_from_vector.cpp | 6 ++++++ r/tests/testthat/test-Array.R | 5 +++++ r/tests/testthat/test-RecordBatch.R | 5 +++++ 3 files changed, 16 insertions(+) diff --git a/r/src/array_from_vector.cpp b/r/src/array_from_vector.cpp index 6c5d0533e5b..784f43da387 100644 --- a/r/src/array_from_vector.cpp +++ b/r/src/array_from_vector.cpp @@ -921,6 +921,12 @@ bool CheckCompatibleFactor(SEXP obj, const std::shared_ptr& typ std::shared_ptr Array__from_vector( SEXP x, const std::shared_ptr& type, bool type_infered) { + + // short circuit if `x` is already an Array + if (Rf_inherits(x, "arrow::Array")) { + return Rcpp::ConstReferenceSmartPtrInputParameter>(x); + } + // special case when we can just use the data from the R vector // directly. This still needs to handle the null bitmap if (arrow::r::can_reuse_memory(x, type)) { diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 68fd9dc04be..fd2fe518631 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -410,3 +410,8 @@ test_that("Array$as_vector() converts to integer (ARROW-3794)", { expect_equal(a$type, uint8()) expect_equal(a$as_vector(), 0:255) }) + +test_that("array() recognise arrow::Array (ARROW-3815)", { + a <- array(1:10) + expect_equal(a, array(a)) +}) diff --git a/r/tests/testthat/test-RecordBatch.R b/r/tests/testthat/test-RecordBatch.R index c0295bac2c8..a492f0f088f 100644 --- a/r/tests/testthat/test-RecordBatch.R +++ b/r/tests/testthat/test-RecordBatch.R @@ -145,3 +145,8 @@ test_that("RecordBatch dim() and nrow() (ARROW-3816)", { expect_equal(dim(batch), c(10L, 2L)) expect_equal(nrow(batch), 10L) }) + +test_that("record_batch() handles arrow::Array", { + batch <- record_batch(x = 1:10, y = arrow::array(1:10)) + expect_equal(batch$schema, schema(x = int32(), y = int32())) +}) From 4fb4f332322433edb98d0d0fc2909fac571f2775 Mon Sep 17 00:00:00 2001 From: Romain Francois Date: Mon, 10 Jun 2019 15:09:53 +0200 Subject: [PATCH 3/5] additional chunked_array() test --- r/tests/testthat/test-chunkedarray.R | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/r/tests/testthat/test-chunkedarray.R b/r/tests/testthat/test-chunkedarray.R index 5e22e5cf31e..9505b8f22a0 100644 --- a/r/tests/testthat/test-chunkedarray.R +++ b/r/tests/testthat/test-chunkedarray.R @@ -265,3 +265,10 @@ test_that("chunked_array() handles 0 chunks if given a type", { expect_equal(a$length(), 0L) } }) + +test_that("chunked_array() can ingest arrays (ARROW-3815)", { + expect_equal( + chunked_array(1:5, array(6:10))$as_vector(), + 1:10 + ) +}) From 9e479c81b661f486fb815495a5cb86c1c3bf5e08 Mon Sep 17 00:00:00 2001 From: Romain Francois Date: Tue, 11 Jun 2019 10:38:29 +0200 Subject: [PATCH 4/5] linting --- r/src/array_from_vector.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/r/src/array_from_vector.cpp b/r/src/array_from_vector.cpp index 784f43da387..0d4e31883f0 100644 --- a/r/src/array_from_vector.cpp +++ b/r/src/array_from_vector.cpp @@ -765,7 +765,8 @@ std::shared_ptr InferType(SEXP x) { switch (TYPEOF(x)) { case ENVSXP: if (Rf_inherits(x, "arrow::Array")) { - Rcpp::ConstReferenceSmartPtrInputParameter> array(x); + Rcpp::ConstReferenceSmartPtrInputParameter> array( + x); return static_cast>(array)->type(); } break; @@ -921,7 +922,6 @@ bool CheckCompatibleFactor(SEXP obj, const std::shared_ptr& typ std::shared_ptr Array__from_vector( SEXP x, const std::shared_ptr& type, bool type_infered) { - // short circuit if `x` is already an Array if (Rf_inherits(x, "arrow::Array")) { return Rcpp::ConstReferenceSmartPtrInputParameter>(x); From 189ab978c8ba1927a3455fcbf7f97b1ea7cc1009 Mon Sep 17 00:00:00 2001 From: Romain Francois Date: Tue, 11 Jun 2019 11:49:01 +0200 Subject: [PATCH 5/5] more idiomatic C++11 range foor loop --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 74b5bad7fe7..36a2dccf666 100644 --- a/.travis.yml +++ b/.travis.yml @@ -362,6 +362,7 @@ matrix: - $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh --only-library - export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$TRAVIS_BUILD_DIR/cpp-install/lib - export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:$TRAVIS_BUILD_DIR/cpp-install/lib/pkgconfig + - export CXX11FLAGS=-Wall - pushd ${TRAVIS_BUILD_DIR}/r after_success: - Rscript ../ci/travis_upload_r_coverage.R