Skip to content

Commit

Permalink
Allow NULL in vec_detect_complete() (#1916)
Browse files Browse the repository at this point in the history
* Allow `NULL` in `vec_detect_complete()`

And better detect `NULL` columns and df-cols in data frames

* NEWS bullet

* Snapshot tests for `vec_rank(NULL)`

* Tweak `.gitignore`
  • Loading branch information
DavisVaughan authored May 14, 2024
1 parent d75f008 commit 585f8b9
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 2 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ docs
.clangd
launch.json
.vscode/
.cache/
compile_commands.json
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# vctrs (development version)

* `vec_detect_complete(NULL)` now returns `logical()`, consistent with
`vec_detect_missing(NULL)` (#1916).

# vctrs 0.6.5

* Internal changes requested by CRAN around C level format strings (#1896).
Expand Down
24 changes: 22 additions & 2 deletions src/complete.c
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ void vec_detect_complete_switch(SEXP x, R_len_t size, int* p_out) {
case VCTRS_TYPE_raw: raw_detect_complete(x, size, p_out); break;
case VCTRS_TYPE_list: list_detect_complete(x, size, p_out); break;
case VCTRS_TYPE_dataframe: df_detect_complete(x, size, p_out); break;
case VCTRS_TYPE_scalar: r_stop_internal("Can't detect missing values in scalars.");
case VCTRS_TYPE_null: break;
case VCTRS_TYPE_scalar: stop_scalar_type(x, vec_args.empty, r_lazy_null);
default: stop_unimplemented_vctrs_type("vec_detect_complete", vec_proxy_typeof(x));
}
}
Expand Down Expand Up @@ -169,12 +170,31 @@ void list_detect_complete(SEXP x, R_len_t size, int* p_out) {

// -----------------------------------------------------------------------------

static inline void col_detect_complete_switch(SEXP x, R_len_t size, int* p_out);

static inline
void df_detect_complete(SEXP x, R_len_t size, int* p_out) {
r_ssize n_cols = r_length(x);
const SEXP* p_x = VECTOR_PTR_RO(x);

for (r_ssize i = 0; i < n_cols; ++i) {
vec_detect_complete_switch(p_x[i], size, p_out);
col_detect_complete_switch(p_x[i], size, p_out);
}
}

static inline
void col_detect_complete_switch(SEXP x, R_len_t size, int* p_out) {
switch (vec_proxy_typeof(x)) {
case VCTRS_TYPE_logical: lgl_detect_complete(x, size, p_out); break;
case VCTRS_TYPE_integer: int_detect_complete(x, size, p_out); break;
case VCTRS_TYPE_double: dbl_detect_complete(x, size, p_out); break;
case VCTRS_TYPE_complex: cpl_detect_complete(x, size, p_out); break;
case VCTRS_TYPE_character: chr_detect_complete(x, size, p_out); break;
case VCTRS_TYPE_raw: raw_detect_complete(x, size, p_out); break;
case VCTRS_TYPE_list: list_detect_complete(x, size, p_out); break;
case VCTRS_TYPE_dataframe: r_stop_internal("Data frame columns should have been flattened by now.");
case VCTRS_TYPE_null: r_abort("Unexpected `NULL` column found in a data frame.");
case VCTRS_TYPE_scalar: stop_scalar_type(x, vec_args.empty, r_lazy_null);
default: stop_unimplemented_vctrs_type("vec_detect_complete", vec_proxy_typeof(x));
}
}
16 changes: 16 additions & 0 deletions tests/testthat/_snaps/complete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# catches `NULL` data frame columns

Code
vec_detect_complete(df)
Condition
Error in `vec_detect_complete()`:
! Unexpected `NULL` column found in a data frame.

# catches scalar objects

Code
vec_detect_complete(lm(1 ~ 1))
Condition
Error in `vec_size()`:
! `x` must be a vector, not a <lm> object.

16 changes: 16 additions & 0 deletions tests/testthat/_snaps/rank.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
# `x` must not be `NULL` (#1823)

Code
vec_rank(NULL)
Condition
Error:
! This type is not supported by `vec_order()`.

---

Code
vec_rank(NULL, incomplete = "na")
Condition
Error:
! This type is not supported by `vec_order()`.

# `ties` is validated

Code
Expand Down
19 changes: 19 additions & 0 deletions tests/testthat/test-complete.R
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,22 @@ test_that("works with arrays", {
expect_identical(vec_detect_complete(x), c(TRUE, FALSE))
expect_identical(vec_detect_complete(y), c(TRUE, FALSE))
})

test_that("works with `NULL`", {
# Consistent with `vec_detect_missing()`
expect_identical(vec_detect_complete(NULL), logical())
})

test_that("catches `NULL` data frame columns", {
df <- new_data_frame(list(x = integer(), y = NULL), n = 0L)

expect_snapshot(error = TRUE, {
vec_detect_complete(df)
})
})

test_that("catches scalar objects", {
expect_snapshot(error = TRUE, {
vec_detect_complete(lm(1 ~ 1))
})
})
9 changes: 9 additions & 0 deletions tests/testthat/test-rank.R
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,15 @@ test_that("`x` must be a vector", {
expect_error(vec_rank(identity), class = "vctrs_error_scalar_type")
})

test_that("`x` must not be `NULL` (#1823)", {
expect_snapshot(error = TRUE, {
vec_rank(NULL)
})
expect_snapshot(error = TRUE, {
vec_rank(NULL, incomplete = "na")
})
})

test_that("`ties` is validated", {
expect_snapshot(error = TRUE, vec_rank(1, ties = "foo"))
expect_snapshot(error = TRUE, vec_rank(1, ties = 1))
Expand Down

0 comments on commit 585f8b9

Please sign in to comment.