[arrow] branch main updated: GH-35952: [R] Ensure that schema metadata can actually be set as a named character vector (#35954)

[thisisnic]

This is an automated email from the ASF dual-hosted git repository.

thisisnic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new fe6c067e69 GH-35952: [R] Ensure that schema metadata can actually be 
set as a named character vector (#35954)
fe6c067e69 is described below

commit fe6c067e690ab28392d19388810725ce9f7ad646
Author: Dewey Dunnington <de...@voltrondata.com>
AuthorDate: Tue Jun 6 13:36:43 2023 -0400

    GH-35952: [R] Ensure that schema metadata can actually be set as a named 
character vector (#35954)
    
    This wasn't necessarily a regression (reprex fails in 12.0.0 as well), 
although the comments suggest that assigning a named character vector will work 
when assigning schema metadata and this feature appears to be used by at least 
one of our dependencies (sfarrow). Given that the sfarrow check passes on 
12.0.0, there is possibly also a place in our code that returns a named 
character vector rather than a list. I've confirmed that this fix solves the 
reverse dependency failure building  [...]
    
    ``` r
    library(arrow, warn.conflicts = FALSE)
    
    schema <- schema(x = int32())
    schema$metadata <- c("name" = "value")
    schema$metadata
    #> $name
    #> [1] "value"
    ```
    
    <sup>Created on 2023-06-06 with [reprex 
v2.0.2](https://reprex.tidyverse.org)</sup>
    * Closes: #35952
    
    Authored-by: Dewey Dunnington <de...@voltrondata.com>
    Signed-off-by: Nic Crane <thisis...@gmail.com>
---
 r/R/schema.R                   |  4 ++++
 r/tests/testthat/test-schema.R | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/r/R/schema.R b/r/R/schema.R
index 9ff38487c4..dc0b4ba81f 100644
--- a/r/R/schema.R
+++ b/r/R/schema.R
@@ -229,9 +229,13 @@ prepare_key_value_metadata <- function(metadata) {
       call. = FALSE
     )
   }
+
+  metadata <- as.list(metadata)
+
   if (!is_empty(metadata) && is.list(metadata[["r"]])) {
     metadata[["r"]] <- .serialize_arrow_r_metadata(metadata[["r"]])
   }
+
   map_chr(metadata, as.character)
 }
 
diff --git a/r/tests/testthat/test-schema.R b/r/tests/testthat/test-schema.R
index 24776e6d0c..a6a0555eac 100644
--- a/r/tests/testthat/test-schema.R
+++ b/r/tests/testthat/test-schema.R
@@ -140,6 +140,22 @@ test_that("Schema modification", {
   expect_error(schm[[c(2, 4)]] <- int32(), "length(i) not equal to 1", fixed = 
TRUE)
 })
 
+test_that("Metadata can be reassigned as a whole", {
+  schm <- schema(b = double(), c = string(), d = int8())
+
+  # Check named character vector
+  schm$metadata <- c("foo" = "bar")
+  expect_identical(schm$metadata, list(foo = "bar"))
+
+  # Check list()
+  schm$metadata <- list("foo" = "bar")
+  expect_identical(schm$metadata, list(foo = "bar"))
+
+  # Check NULL for removal
+  schm$metadata <- NULL
+  expect_identical(schm$metadata, set_names(list(), character()))
+})
+
 test_that("Metadata is preserved when modifying Schema", {
   schm <- schema(b = double(), c = string(), d = int8())
   schm$metadata$foo <- "bar"