pluck(x, NULL) errs; use cases for why it shouldn't?

[mmuurr]

This is tangentially-related to #480, but is manifested in a slightly different way, so I figured to open a new issue. Currently both of these throw errors:

pluck(x, NULL)
pluck(x, character(0))

I think this might be too harsh.

In #480, @jennybc mentions:

It does seem sort of troubling that pluck() won't allow you to retain, say, character(0) vs NULL inside some workflow. Yet I don't have any current compelling example where this is harmful.

Workflow Example 1

One such use case (where NULL and/or character(0) are part of a workflow) is in Shiny apps. shiny::radioButtons, e.g., can be configured to have no default selection, in which case the reactive value server-side is NULL. If those radio buttons are used to pluck a dataset out of a list, the app author now needs to add additional code (admittedly, perhaps not a lot of additional code by using shiny::req) to check for the un-set case. Adding this additional code for some app improvement is great; needing to add additional code to prevent a pluck error feels less great, especially since pluck already has .default semantics. In the app case, for example, one might want to set .default to hard-coded example dataset for the rest of the app to use on initialization:

output$some_output <- renderSomething({
  pluck(my_data, input$my_radio_buttons, .default = my_default_data_set) %>%
    downstream_processing()
})

^ That fails when input$my_radio_buttons is NULL. Adding additional req or isTruthy (or equivalent) logic simply to avoid the pluck error when .default is present seems cumbersome.

Workflow Example 2

Another workflow example is simply using pluck in one list to retrieve a value that is then used as a key when pluck-ing from another list:

obj1 <- list(my_type = "type1", some = "other", data = "here")
obj2 <- list(
  foo = list(  ## just for the sake of inserting a nested list to demonstrate pluck's utility
    "type1" = c(some, stuff, here),
    "type2" = c(some, other, stuff, here)
  )
)
## let's retrieve my_type from obj1 and use that to fetch stuff from obj2:
pluck(obj2, "foo", pluck(obj1, "my_type"), .default = c(some, default, stuff))

^ That works great when "my_type" is present in obj1.
But if "my_type" is missing, the outer pluck throws an error because pluck(obj1, "my_type") resolves to NULL. So we have to add conditionals to short-circuit the outer pluck call, returning the .default in that case and keeping the .default in the outer pluck in the instances where "my_type" is present in obj1 but doesn't have a corresponding value in obj2.

In both such examples, there're clearly ways around this, mostly with lots of conditionals (or wrapping pluck in tryCatch), but the .default option in pluck feels like it was introduced to handle non-existent keys, and NULL feels like a non-existent key. Probably the same reasoning holds for character(0) (and other zero-length values passed as keys), but I can definitely see throwing errors when passed a length > 1 vector as a pluck element key. Likewise, chuck() should throw an error when given NULL as a key, as it's advertised as being strict.

[mmuurr]

As a follow-on, I've also noticed that integer(0) as a key works, which seems inconsistent w.r.t. both character(0) and NULL ... all three of those yield either different results of different error messages:

library(purrr)
l <- list(x = 1:3)
pluck(l, character(0))
#> Error in pluck(l, character(0)): CHAR() can only be applied to a 'CHARSXP', not a 'NULL'
pluck(l, integer(0))
#> NULL
pluck(l, NULL)
#> Error: Index 1 must be a character or numeric vector, not a list

(purrr v0.3.4)

[hadley]

This is unrelated to #480.

But you don't actually propose what you think pluck(x, NULL) should do?

[hadley]

We're currently a bit inconsistent, and the first error message isn't useful.

library(purrr)

pluck(NULL, NULL)
#> NULL
pluck(list(), NULL)
#> NULL
pluck(1, NULL)
#> Error in `stop_bad_type()` at purrr/R/conditions.R:71:2:
#> ! Index 1 must be a character or numeric vector, not a double vector

pluck(NULL, integer())
#> NULL
pluck(list(), integer())
#> NULL
pluck(1, integer())
#> NULL

pluck(NULL, 1:2)
#> NULL
pluck(list(), 1:2)
#> NULL
pluck(1, 1:2)
#> Error in `stop_bad_length()` at purrr/R/conditions.R:165:2:
#> ! Index 1 must have length 1, not 2

^{Created on 2022-08-27 by the reprex package (v2.0.1)}

Seems like we need to consistently validate the length of the index.