PostcodesioR icon indicating copy to clipboard operation
PostcodesioR copied to clipboard

Feature request: bulk_postcode_lookup should accept a character vector and/or `...`

Open dave-lovell opened this issue 1 year ago • 2 comments

bulk_postcode_lookup() requires postcodes to be a length-one list that contains only a character vector, as exemplified in the function's help page:

pc_list <- list(postcodes = c("PR3 0SG", "M45 6GN", "EX165BL"))
bulk_postcode_lookup(pc_list)

I'm continually caught off-guard by this counter-intuitive requirement. What is the purpose of the list?

Please implement support for accepting a character vector and/or ..., like so:

bulk_postcode_lookup(c("PR3 0SG", "M45 6GN", "EX165BL"))
bulk_postcode_lookup("PR3 0SG", "M45 6GN", "EX165BL")

Those inputs could be coerced into list form under the hood so that the functionality of old code is preserved.

dave-lovell avatar Mar 05 '24 11:03 dave-lovell

Thanks for the suggestion @dave-lovell This is a legacy issue and it can be sorted quite quickly by adjusting check_list_limit() within bulk_postcode_lookup(). Please feel from to submit a PR. If not, I might be able to fix it over the weekend.

erykwalczak avatar Mar 05 '24 12:03 erykwalczak

@erzk I've requested this in https://github.com/ropensci/PostcodesioR/pull/19. I've left check_list_limit() pretty much untouched (save the wording of the error messages) and instead opted to concatenate all arguments to the function before recursively unlisting them and finally wrapping them into a list:

bulk_postcode_lookup <- function(...) {
  dots <- unlist(c(...), recursive = TRUE)
  postcodes <- list(postcodes = dots)
  check_list_limit(postcodes)
#...

This should mean that legacy code still works as expected.

dave-lovell avatar Mar 05 '24 12:03 dave-lovell