Skip to content

ARROW-11832: [R] Handle conversion of extra nested struct column #9919

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.

Sign up for GitHub

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

Closed
romainfrancois wants to merge 2 commits into from
Closed

ARROW-11832: [R] Handle conversion of extra nested struct column #9919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
161b37b
layer about vctrs::::short_vec_size()
romainfrancois Apr 7, 2021
24232d1
test
romainfrancois Apr 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion r/src/arrow_vctrs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@
#pragma once

namespace vctrs {
R_len_t short_vec_size(SEXP);
R_len_t vec_size(SEXP);
}
8 changes: 7 additions & 1 deletion r/src/imports.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ const vctrs_api_ptrs_t& vctrs_api() {
return ptrs;
}

R_len_t short_vec_size(SEXP x) { return vctrs_api().short_vec_size(x); }
R_len_t vec_size(SEXP x) {
if (Rf_inherits(x, "data.frame") || TYPEOF(x) != VECSXP || Rf_inherits(x, "POSIXlt")) {
return vctrs_api().short_vec_size(x);
} else {
return Rf_length(x);
}
}

} // namespace vctrs
14 changes: 3 additions & 11 deletions r/src/r_to_arrow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,15 +809,7 @@ class RListConverter : public ListConverter<T, RConverter, RConverterTrait> {
}

auto append_value = [this](SEXP value) {
// TODO: this should always use vctrs::short_vec_size
// but that introduced a regression:
// https://github.com/apache/arrow/pull/8650#issuecomment-786940734
int n;
if (TYPEOF(value) == VECSXP && !Rf_inherits(value, "data.frame")) {
n = Rf_length(value);
} else {
n = vctrs::short_vec_size(value);
}
int n = vctrs::vec_size(value);

RETURN_NOT_OK(this->list_builder_->ValidateOverflow(n));
RETURN_NOT_OK(this->list_builder_->Append());
Expand Down Expand Up @@ -879,7 +871,7 @@ class RStructConverter : public StructConverter<RConverter, RConverterTrait> {

for (R_xlen_t i = 0; i < n_columns; i++) {
SEXP x_i = VECTOR_ELT(x, i);
if (vctrs::short_vec_size(x_i) < size) {
if (vctrs::vec_size(x_i) < size) {
return Status::RError("Degenerated data frame");
}
}
Expand Down Expand Up @@ -1011,7 +1003,7 @@ std::shared_ptr<arrow::Array> vec_to_arrow(SEXP x,
RConversionOptions options;
options.strict = !type_inferred;
options.type = type;
options.size = vctrs::short_vec_size(x);
options.size = vctrs::vec_size(x);

// maybe short circuit when zero-copy is possible
if (can_reuse_memory(x, options.type)) {
Expand Down
5 changes: 5 additions & 0 deletions r/tests/testthat/test-Array.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,11 @@ test_that("Array$create() handles data frame -> struct arrays (ARROW-3811)", {
a <- Array$create(df)
expect_type_equal(a$type, struct(x = int32(), y = float64(), z = utf8()))
expect_equivalent(as.vector(a), df)

df <- structure(list(col = structure(list(structure(list(list(structure(1))), class = "inner")), class = "outer")), class = "data.frame", row.names = c(NA, -1L))
a <- Array$create(df)
expect_type_equal(a$type, struct(col = list_of(list_of(list_of(float64())))))
expect_equivalent(as.vector(a), df)
})

test_that("StructArray methods", {
Expand Down