From e6570fa36b1bcf6b09f9ceb2577f66a2575aed86 Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Mon, 3 Apr 2023 14:54:42 +0100 Subject: [PATCH 1/6] Lint away... --- r/tests/testthat/test-Table.R | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/r/tests/testthat/test-Table.R b/r/tests/testthat/test-Table.R index 817b645fad9..3bd42399d76 100644 --- a/r/tests/testthat/test-Table.R +++ b/r/tests/testthat/test-Table.R @@ -711,3 +711,14 @@ test_that("as_arrow_table() errors on data.frame with NULL names", { names(df) <- NULL expect_error(as_arrow_table(df), "Input data frame columns must be named") }) + +test_that("as.data.frame() on an ArrowTabular object returns a vanilla data.frame and not a tibble", { + df <- data.frame(x = 1) + out1 <- as.data.frame(arrow::arrow_table(df, name = "1")) + out2 <- as.data.frame(arrow::arrow_table(name = "1", df)) + out3 <- as.data.frame(arrow::arrow_table(df)) + + expect_s3_class(out1, "data.frame", exact = TRUE) + expect_s3_class(out2, "data.frame", exact = TRUE) + expect_s3_class(out3, "data.frame", exact = TRUE) +}) From 06ef53dca6928db080a9da538ba0ae8bd805df41 Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Fri, 31 Mar 2023 14:19:27 +0100 Subject: [PATCH 2/6] Only preserve metadata if input df is 1 item long --- r/src/table.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/src/table.cpp b/r/src/table.cpp index 498141cc2f2..5691f3c1a15 100644 --- a/r/src/table.cpp +++ b/r/src/table.cpp @@ -228,7 +228,7 @@ arrow::Status AddMetadataFromDots(SEXP lst, int num_fields, // "top level" attributes, only relevant if the first object is not named and a data // frame cpp11::strings names = Rf_getAttrib(lst, R_NamesSymbol); - if (names[0] == "" && Rf_inherits(VECTOR_ELT(lst, 0), "data.frame")) { + if (names[0] == "" && Rf_inherits(VECTOR_ELT(lst, 0), "data.frame") && Rf_xlength(lst) == 1) { SEXP top_level = metadata[0] = arrow_attributes(VECTOR_ELT(lst, 0), true); if (!Rf_isNull(top_level) && XLENGTH(top_level) > 0) { has_top_level_metadata = true; From 8d69f3e637b83e910d138e99ce3c9fd09ce970e8 Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Fri, 31 Mar 2023 14:35:49 +0100 Subject: [PATCH 3/6] Run linter --- r/src/table.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/r/src/table.cpp b/r/src/table.cpp index 5691f3c1a15..04537000f5d 100644 --- a/r/src/table.cpp +++ b/r/src/table.cpp @@ -228,7 +228,8 @@ arrow::Status AddMetadataFromDots(SEXP lst, int num_fields, // "top level" attributes, only relevant if the first object is not named and a data // frame cpp11::strings names = Rf_getAttrib(lst, R_NamesSymbol); - if (names[0] == "" && Rf_inherits(VECTOR_ELT(lst, 0), "data.frame") && Rf_xlength(lst) == 1) { + if (names[0] == "" && Rf_inherits(VECTOR_ELT(lst, 0), "data.frame") && + Rf_xlength(lst) == 1) { SEXP top_level = metadata[0] = arrow_attributes(VECTOR_ELT(lst, 0), true); if (!Rf_isNull(top_level) && XLENGTH(top_level) > 0) { has_top_level_metadata = true; From 01dcd08960284fc14cc8a1bc351ac06ac8ade96b Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Tue, 11 Apr 2023 14:42:48 +0100 Subject: [PATCH 4/6] Update tests to better reflect changes --- r/tests/testthat/test-Table.R | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/r/tests/testthat/test-Table.R b/r/tests/testthat/test-Table.R index 3bd42399d76..0e464ab8a0f 100644 --- a/r/tests/testthat/test-Table.R +++ b/r/tests/testthat/test-Table.R @@ -712,13 +712,19 @@ test_that("as_arrow_table() errors on data.frame with NULL names", { expect_error(as_arrow_table(df), "Input data frame columns must be named") }) -test_that("as.data.frame() on an ArrowTabular object returns a vanilla data.frame and not a tibble", { +test_that("we only preserve metadata of input to arrow_table when passed a single data.frame", { + # data.frame in, data.frame out df <- data.frame(x = 1) - out1 <- as.data.frame(arrow::arrow_table(df, name = "1")) - out2 <- as.data.frame(arrow::arrow_table(name = "1", df)) - out3 <- as.data.frame(arrow::arrow_table(df)) - + out1 <- as.data.frame(arrow_table(df)) expect_s3_class(out1, "data.frame", exact = TRUE) - expect_s3_class(out2, "data.frame", exact = TRUE) - expect_s3_class(out3, "data.frame", exact = TRUE) + + # tibble in, tibble out + tib <- tibble::tibble(x = 1) + out2 <- as.data.frame(arrow_table(tib)) + expect_s3_class(out2, c("tbl_df", "tbl", "data.frame"), exact = TRUE) + + # GH-35038 - passing in multiple arguments doesn't affect return type + out3 <- as.data.frame(arrow::arrow_table(df, name = "1")) + out4 <- as.data.frame(arrow::arrow_table(name = "1", df)) + expect_identical(class(out3), class(out4)) }) From 499c638d724c3b7527fff36814a4c62ef123b794 Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Tue, 11 Apr 2023 16:06:57 +0100 Subject: [PATCH 5/6] Update r/tests/testthat/test-Table.R Co-authored-by: Neal Richardson --- r/tests/testthat/test-Table.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/r/tests/testthat/test-Table.R b/r/tests/testthat/test-Table.R index 0e464ab8a0f..fce80a4bdaf 100644 --- a/r/tests/testthat/test-Table.R +++ b/r/tests/testthat/test-Table.R @@ -724,7 +724,7 @@ test_that("we only preserve metadata of input to arrow_table when passed a singl expect_s3_class(out2, c("tbl_df", "tbl", "data.frame"), exact = TRUE) # GH-35038 - passing in multiple arguments doesn't affect return type - out3 <- as.data.frame(arrow::arrow_table(df, name = "1")) - out4 <- as.data.frame(arrow::arrow_table(name = "1", df)) + out3 <- as.data.frame(arrow_table(df, name = "1")) + out4 <- as.data.frame(arrow_table(name = "1", df)) expect_identical(class(out3), class(out4)) }) From e5dfdc6fdf69e9121ba86e670bd109bd149b7123 Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Tue, 11 Apr 2023 16:08:56 +0100 Subject: [PATCH 6/6] Test actual types --- r/tests/testthat/test-Table.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/r/tests/testthat/test-Table.R b/r/tests/testthat/test-Table.R index fce80a4bdaf..233705323e7 100644 --- a/r/tests/testthat/test-Table.R +++ b/r/tests/testthat/test-Table.R @@ -726,5 +726,7 @@ test_that("we only preserve metadata of input to arrow_table when passed a singl # GH-35038 - passing in multiple arguments doesn't affect return type out3 <- as.data.frame(arrow_table(df, name = "1")) out4 <- as.data.frame(arrow_table(name = "1", df)) - expect_identical(class(out3), class(out4)) + + expect_s3_class(out3, c("tbl_df", "tbl", "data.frame"), exact = TRUE) + expect_s3_class(out4, c("tbl_df", "tbl", "data.frame"), exact = TRUE) })