cmu-delphi / epiprocess

Tools for basic signal processing in epidemiology
https://cmu-delphi.github.io/epiprocess/
Other
13 stars 9 forks source link

Check for `NULL` slide computation outputs #292

Closed brookslogan closed 2 days ago

brookslogan commented 1 year ago

E.g., invisible(NULL) from forgetting to return a value from a computation where the last statement was some imperative-like computation that returned invisible(NULL). Or from users thinking they can return NULL to contribute 0 rows to the result that way. We should give a comprehensible error message rather than the one below. (Later, we could consider assigning NULL this special meaning no-rows-in-the-result meaning, but this probably requires more thought about future interface updates and dplyr incompatibilities.)

library(dplyr)
#> 
#> Attaching package: 'dplyr'
#> The following objects are masked from 'package:stats':
#> 
#>     filter, lag
#> The following objects are masked from 'package:base':
#> 
#>     intersect, setdiff, setequal, union
library(epiprocess)
#> 
#> Attaching package: 'epiprocess'
#> The following object is masked from 'package:stats':
#> 
#>     filter
ea = tibble::tribble(~version, ~time_value, ~binary,
                     2,        1:1,          2^(1:1),
                     3,        1:2,          2^(2:1),
                     4,        1:3,          2^(3:1),
                     5,        1:4,          2^(4:1),
                     6,        1:5,          2^(5:1),
                     7,        1:6,          2^(6:1)) %>%
  tidyr::unnest(c(time_value,binary)) %>%
  mutate(geo_value = "x") %>%
  as_epi_archive()
  ea %>%
    epix_slide(before = 5L, function(x, g) {
    })
#> Error in `tidyr::unnest()` at epiprocess/R/grouped_epi_archive.R:429:14:
#> ! Can't subset columns that don't exist.
#> ✖ Column `slide_value` doesn't exist.

#> Backtrace:
#>      ▆
#>   1. ├─`%>%`(...)
#>   2. ├─epiprocess::epix_slide(...)
#>   3. │ └─x$slide(...) at epiprocess/R/methods-epi_archive.R:853:2
#>   4. │   ├─... %>% ungroup() at epiprocess/R/archive.R:647:12
#>   5. │   └─self$group_by()$slide(...)
#>   6. │     ├─tidyr::unnest(x, !!new_col, names_sep = names_sep) at epiprocess/R/grouped_epi_archive.R:429:14
#>   7. │     └─tidyr:::unnest.data.frame(x, !!new_col, names_sep = names_sep)
#>   8. │       └─tidyselect::eval_select(enquo(cols), data)
#>   9. │         └─tidyselect:::eval_select_impl(...)
#>  10. │           ├─tidyselect:::with_subscript_errors(...)
#>  11. │           │ └─rlang::try_fetch(...)
#>  12. │           │   └─base::withCallingHandlers(...)
#>  13. │           └─tidyselect:::vars_select_eval(...)
#>  14. │             └─tidyselect:::walk_data_tree(expr, data_mask, context_mask)
#>  15. │               └─tidyselect:::as_indices_sel_impl(...)
#>  16. │                 └─tidyselect:::as_indices_impl(...)
#>  17. │                   └─tidyselect:::chr_as_locations(x, vars, call = call, arg = arg)
#>  18. │                     └─vctrs::vec_as_location(...)
#>  19. ├─dplyr::ungroup(.)
#>  20. └─vctrs (local) `<fn>`()
#>  21.   └─vctrs:::stop_subscript_oob(...)
#>  22.     └─vctrs:::stop_subscript(...)
#>  23.       └─rlang::abort(...)

Created on 2023-03-28 with reprex v2.0.2

brookslogan commented 1 year ago

We should either raise an error or assign NULL a particular meaning.

brookslogan commented 2 days ago

Handled in 9.0.