tidyverse / funs

Collection of low-level functions for working with vctrs
Other
34 stars 7 forks source link

nth(), first() and last() #56

Open romainfrancois opened 4 years ago

romainfrancois commented 4 years ago

closes #35

This is for now similar to the versions in dplyr except that it does not have order_by=. Should it ?

Also wondering:

Apart from that, this is mostly a wrapper around vec_slice():

library(funs, warn.conflicts = FALSE)

nth(letters, 1)
#> [1] "a"
nth(letters, -1)
#> [1] "z"
nth(letters, 30)
#> [1] NA
nth(character(), 2)
#> [1] NA

df <- data.frame(x = 1:10, y = letters[1:10])
nth(df, 3)
#>   x y
#> 1 3 c
nth(df, -2)
#>   x y
#> 1 9 i
nth(df, 11)
#>    x    y
#> 1 NA <NA>

mat <- matrix(1:20, ncol = 4)
nth(mat, 2)
#>      [,1] [,2] [,3] [,4]
#> [1,]    2    7   12   17
nth(mat, -1)
#>      [,1] [,2] [,3] [,4]
#> [1,]    5   10   15   20

lst <- list(1, letters, TRUE)
nth(lst, 1)
#> [[1]]
#> [1] 1
nth(lst, 4)
#> [[1]]
#> NULL

Created on 2020-09-03 by the reprex package (v0.3.0.9001)

hadley commented 4 years ago

I think we have to keep supporting negative values in order to not break existing dplyr code. And I'm pretty sure we have to return the default value for OOB values, since that's the primary value of these functions over [[.

@DavisVaughan do you have any thoughts on whether we should support order_by here?

romainfrancois commented 3 years ago

Is this going to be annoying ?

library(funs, warn.conflicts = FALSE)

x <- 1:4
nth(x, 6) <- 5
#> Warning: Unchanged `x` as `n` is out of bounds
x
#> [1] 1 2 3 4

Created on 2021-05-17 by the reprex package (v2.0.0)

lionel- commented 3 years ago

I think we generally prefer errors to warnings? I.e. the behaviour should either be silent or an error? The reason is that this sort of warnings are never desirable for end users and so should be considered programming errors.

In this case it seems OOB assignment should be an error? I think it's ok to have a lax nth() and strict nth<-. Or we could add an oob = c("error", "ignore") argument:

nth(x, 2, oob = "ignore") <- 10
romainfrancois commented 3 years ago

Should both n tests be grouped together and maybe hint more, something like:

  if (!is_integerish(n, n = 1L, finite = TRUE)) {
    abort("`n` must be a single integer.")
  }
  if (n == 0 || n > size || n < -size) {
    abort(c(
      "`n` is out of bounds.",
      x = glue::glue("`n = {n}` cannot be used to modify a vector of size {size}."),
      i = glue::glue("Use a positive integer in [1,{size}] to select from the left."),
      i = glue::glue("Use a negative integer in [-{size}, -1] to select from the right.")
    ))
  }

This should maybe also abort if size is 0