Merge pull request #1511 from tidyverse/f-741-924-vec-as-location-2

R/sub.R

@@ -175,10 +175,6 @@ NULL
   } else {
     j <- vectbl_as_col_location(j, length(x), names(x), j_arg = j_arg, assign = FALSE)
 
-    if (anyNA(j)) {
-      abort_na_column_index(which(is.na(j)))
-    }
-
     xo <- .subset(x, j)
 
     if (anyDuplicated.default(j)) {
@@ -251,11 +247,19 @@ vectbl_as_col_subscript2 <- function(j, j_arg, assign = FALSE, call = caller_env
 }
 
 vectbl_as_col_location2 <- function(j, n, names = NULL, j_arg, assign = FALSE, call = caller_env()) {
-  subclass_col_index_errors(vec_as_location2(j, n, names, call = call), j_arg = j_arg, assign = assign)
+  subclass_col_index_errors(
+    vec_as_location2(j, n, names, call = call),
+    j_arg = j_arg,
+    assign = assign
+  )
 }
 
 vectbl_as_row_location2 <- function(i, n, i_arg, assign = FALSE, call = caller_env()) {
-  subclass_row_index_errors(vec_as_location2(i, n, call = call), i_arg = i_arg, assign = assign)
+  subclass_row_index_errors(
+    vec_as_location2(i, n, call = call),
+    i_arg = i_arg,
+    assign = assign
+  )
 }
 
 vectbl_set_names <- function(x, names = NULL) {
@@ -274,7 +278,7 @@ vectbl_as_col_location <- function(j,
                                    assign = FALSE,
                                    call = caller_env()) {
   subclass_col_index_errors(
-    vec_as_location(j, n, names, call = call),
+    vec_as_location(j, n, names, missing = "error", call = call),
     j_arg = j_arg,
     assign = assign
   )
@@ -369,10 +373,6 @@ vectbl_restore <- function(xo, x) {
 
 # Errors ------------------------------------------------------------------
 
-abort_na_column_index <- function(j, call = caller_env()) {
-  tibble_abort(call = call, pluralise_commas("Can't use NA as column index with `[` at position(s) ", j, "."), j = j)
-}
-
 abort_subset_columns_non_missing_only <- function(call = caller_env()) {
   tibble_abort(call = call, "Subscript can't be missing for tibbles in `[[`.")
 }

R/subassign.R

@@ -174,10 +174,6 @@ vectbl_as_new_col_index <- function(j, x, j_arg, names = "", value_arg = NULL, c
   } else {
     j <- vectbl_as_col_location(j, length(x), names(x), j_arg = j_arg, assign = TRUE, call = call)
 
-    if (anyNA(j)) {
-      abort_na_column_index(which(is.na(j)), call)
-    }
-
     if (length(names) != 1L) {
       # Side effect: check compatibility
       vec_recycle(names, length(j), x_arg = as_label(value_arg), call = call)
@@ -230,7 +226,11 @@ vectbl_as_row_location <- function(i, n, i_arg, assign = FALSE, call) {
     i <- i[, 1]
   }
 
-  subclass_row_index_errors(vec_as_location(i, n, call = call), i_arg = i_arg, assign = assign)
+  subclass_row_index_errors(
+    vec_as_location(i, n, missing = if (assign) "error" else "propagate", call = call),
+    i_arg = i_arg,
+    assign = assign
+  )
 }
 
 numtbl_as_col_location_assign <- function(j, n, j_arg, call) {

tests/testthat/_snaps/msg.md

@@ -244,12 +244,6 @@
       <error/tibble_error_need_rhs_vector_or_null>
       Error:
       ! `RHS` must be a vector, a bare list, a data frame, a matrix, or NULL.
-    Code
-      print_error(abort_na_column_index(1:3))
-    Output
-      <error/tibble_error_na_column_index>
-      Error:
-      ! Can't use NA as column index with `[` at positions 1, 2, and 3.
     Code
       print_error(abort_dim_column_index(as.matrix("x")))
     Output

tests/testthat/_snaps/subsetting.md

@@ -72,7 +72,9 @@
       foo[c(1:3, NA)]
     Condition
       Error in `foo[c(1:3, NA)]`:
-      ! Can't use NA as column index with `[` at position 4.
+      ! Can't subset columns with `c(1:3, NA)`.
+      x Subscript `c(1:3, NA)` can't contain missing values.
+      x It has a missing value at location 4.
     Code
       foo[as.matrix(1)]
     Condition
@@ -173,7 +175,9 @@
       foo[c(TRUE, TRUE, NA)]
     Condition
       Error in `foo[c(TRUE, TRUE, NA)]`:
-      ! Can't use NA as column index with `[` at position 3.
+      ! Can't subset columns with `c(TRUE, TRUE, NA)`.
+      x Subscript `c(TRUE, TRUE, NA)` can't contain missing values.
+      x It has a missing value at location 3.
     Code
       foo[as.matrix(TRUE)]
     Condition
@@ -490,12 +494,16 @@
       df[NA] <- 3
     Condition
       Error in `[<-`:
-      ! Can't use NA as column index with `[` at positions 1 and 2.
+      ! Can't assign columns with `NA`.
+      x Subscript `NA` can't contain missing values.
+      x It has a missing value at location 1.
     Code
       df[NA, ] <- 3
     Condition
       Error in `[<-`:
-      ! Can't use NA as row index in a tibble for assignment.
+      ! Can't assign rows with `NA`.
+      x Subscript `NA` can't contain missing values.
+      x It has a missing value at location 1.
     Code
       # # [<-.tbl_df and logical indexes
       df <- tibble(x = 1:2, y = x)

tests/testthat/_snaps/vignette-invariants/invariants.md

@@ -2364,7 +2364,9 @@ with_df(df[NA] <- list("x"))
 ```r
 with_tbl(tbl[NA] <- list("x"))
 #> Error in `[<-`:
-#> ! Can't use NA as column index with `[` at positions 1, 2, and 3.
+#> ! Can't assign columns with `NA`.
+#> x Subscript `NA` can't contain missing values.
+#> x It has a missing value at location 1.
 ```
 
 </td></tr><tr style="vertical-align:top"><td>
@@ -3313,7 +3315,9 @@ with_df(df[NA, ] <- df[1, ])
 ```r
 with_tbl(tbl[NA, ] <- tbl[1, ])
 #> Error in `[<-`:
-#> ! Can't use NA as row index in a tibble for assignment.
+#> ! Can't assign rows with `NA`.
+#> x Subscript `NA` can't contain missing values.
+#> x It has a missing value at location 1.
 ```
 
 </td></tr></tbody></table>

tests/testthat/test-msg.R

@@ -69,7 +69,6 @@ test_that("output test", {
     print_error(abort_need_rhs_vector(quote(RHS)))
     print_error(abort_need_rhs_vector_or_null(quote(RHS)))
 
-    print_error(abort_na_column_index(1:3))
     print_error(abort_dim_column_index(as.matrix("x")))
 
     print_error(abort_assign_columns_non_na_only())

tests/testthat/test-subsetting.R

@@ -129,9 +129,9 @@ test_that("[.tbl_df is careful about column indexes (#83)", {
       foo[-4],
       class = "vctrs_error_subscript_oob"
     )
-    expect_tibble_abort(
+    expect_error(
       foo[c(1:3, NA)],
-      abort_na_column_index(4)
+      class = "vctrs_error_subscript_type"
     )
 
     expect_error(foo[as.matrix(1)])
@@ -156,9 +156,9 @@ test_that("[.tbl_df is careful about column flags (#83)", {
       foo[c(TRUE, TRUE, FALSE, FALSE)],
       class = "vctrs_error_subscript_size"
     )
-    expect_tibble_abort(
+    expect_error(
       foo[c(TRUE, TRUE, NA)],
-      abort_na_column_index(3)
+      class = "vctrs_error_subscript_type"
     )
 
     expect_tibble_abort(