Skip to content

Commit

Permalink
GH-35952: [R] Ensure that schema metadata can actually be set as a na…
Browse files Browse the repository at this point in the history
…med 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 the sfarrow example vignette.

``` 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 <[email protected]>
Signed-off-by: Nic Crane <[email protected]>
  • Loading branch information
paleolimbot authored and thisisnic committed Jun 13, 2023
1 parent c3661d4 commit caf1b98
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 0 deletions.
4 changes: 4 additions & 0 deletions r/R/schema.R
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,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)
}

Expand Down
16 changes: 16 additions & 0 deletions r/tests/testthat/test-schema.R
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit caf1b98

Please sign in to comment.