[help] vctrs and tar_group clashing when using data.table::as.IDate

[robitalec]

Help

• I understand and agree to https://books.ropensci.org/targets/help.html.

Description

When using iteration = 'group' with a data.table (as in #395), if the data.table has an IDate column, tar_read throws the following {vctrs} error:

Reprex:

library(targets)
library(data.table)

tar_dir({
  tar_script(
    list(
      tar_target(
        target_a,
        data.table::data.table(tar_group = c(1, 2, 3)),
        format = "fst_dt",
        iteration = 'group'
      ),
      tar_target(
        target_b,
        target_a[, b := rep(data.table::as.IDate('2015-01-01'), .N)],
        pattern = map(target_a),
        format = "fst_dt"
      )
    ),
    ask = FALSE
  )
  tar_make()
  tar_read(target_b)
})
#> ▶ start target target_a
#> ● built target target_a [0.002 seconds]
#> ▶ start branch target_b_1fddc332
#> ● built branch target_b_1fddc332 [0.002 seconds]
#> ▶ start branch target_b_2f5eb602
#> ● built branch target_b_2f5eb602 [0.001 seconds]
#> ▶ start branch target_b_7efedeb8
#> ● built branch target_b_7efedeb8 [0.001 seconds]
#> ● built pattern target_b
#> ▶ end pipeline [0.186 seconds]
#> Error:
#> ! Can't use a name specification with non-vctrs types.
#> vctrs methods must be implemented for class `IDate`.
#> See <https://vctrs.r-lib.org/articles/s3-vector.html>.
#> Backtrace:
#>      ▆
#>   1. ├─targets::tar_read(target_b)
#>   2. │ └─targets::tar_read_raw(...)
#>   3. │   └─targets:::tar_read_inner(name, branches, meta, path_store = store)
#>   4. │     ├─targets:::if_any(...)
#>   5. │     └─targets:::read_pattern(name, record, meta, branches, path_store)
#>   6. │       ├─targets:::value_produce_aggregate(value, objects)
#>   7. │       └─targets:::value_produce_aggregate.tar_vector(value, objects)
#>   8. │         ├─base::do.call(vctrs::vec_c, objects)
#>   9. │         └─vctrs (local) `<fn>`(...)
#>  10. └─rlang::abort(message = message, call = call)

^{Created on 2023-08-31 with reprex v2.0.2}

It seems to be originating here:

targets/R/class_vector.R

Line 17 in c08ede2

objects$.name_spec <- "{outer}_{inner}"

My current workaround for this specific case is to use iteration = 'list' as a default and combine the list of branches with data.table::rbindlist. This is ok - but a bit clunky.

I ran the tests on a modified version of targets with the L17 above removed and got [ FAIL 2 | WARN 0 | SKIP 496 | PASS 2762 ]. Error 1 = missing the {paws.storage} package, error 2 = related to this change:

Error (test-class_pattern.R:759:3): aggregate names of branches with length > 1 (#320)
Error: Error running targets::tar_make()
  Error messages: targets::tar_meta(fields = error, complete_only = TRUE)
  Debugging guide: https://books.ropensci.org/targets/debugging.html
  How to ask for help: https://books.ropensci.org/targets/help.html
  Last error: could not load dependencies of target z. Can't merge the outer name `y_29239c8a` with a vector of length > 1.
Please supply a `.name_spec` specification.
Backtrace:
     ▆
  1. ├─testthat::expect_silent(tar_make(callr_function = NULL, reporter = "silent")) at test-class_pattern.R:759:2
  2. │ └─testthat:::quasi_capture(enquo(object), NULL, evaluate_promise)
  3. │   ├─testthat (local) .capture(...)
  4. │   │ ├─withr::with_output_sink(...)
  5. │   │ │ └─base::force(code)
  6. │   │ ├─base::withCallingHandlers(...)
  7. │   │ └─base::withVisible(code)
  8. │   └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
  9. └─targets::tar_make(callr_function = NULL, reporter = "silent")
 10.   └─targets:::callr_outer(...) at targets/R/tar_make.R:162:2
 11.     ├─targets:::if_any(...) at targets/R/utils_callr.R:25:2
 12.     └─targets:::callr_error(condition = out, fun = fun) at targets/R/utils_logic.R:35:4
 13.       └─targets::tar_throw_run(message, class = class(condition)) at targets/R/utils_callr.R:45:2
 14.         └─targets::tar_error(...) at targets/R/utils_condition.R:56:2
 15.           └─rlang::abort(message = message, class = class, call = tar_empty_envir) at targets/R/utils_condition.R:119:2

I can certainly move this question over to the {vctrs} issues - I just wanted to check with you first if the use of name_spec here is required, which it seems it might be!

Thank you, and as always - so grateful for your work with this incredible package.

[wlandau]

As you say, iteration = "list" should work for now.

It is tricky to make iteration = "vector" aggregation fully general. The code is here:

targets/R/class_vector.R

Lines 16 to 19 in 0219806

	value_produce_aggregate.tar_vector <- function(value, objects) { # nolint
	objects$.name_spec <- "{outer}_{inner}"
	do.call(vctrs::vec_c, objects)
	}

Sometimes vctrs::vec_c() name spec is required:

vctrs::vec_c(a = c(1, 2), b = c(3, 4, 5))
#> Error in `vctrs::vec_c()`:
#> ! Can't merge the outer name `a` with a vector of length > 1.
#> Please supply a `.name_spec` specification.

But other times, as in your case, the name spec causes problems:

vctrs::vec_c(
  a = data.table::as.IDate("2015-01-01"),
  b = data.table::as.IDate("2015-01-01"),
  .name_spec <- "{outer}_{inner}"
)
#> Error in `vctrs::vec_c()`:
#> ! Can't combine `a` <vctrs:::common_class_fallback> and `..3` <character>.

What's more, removing names altogether causes problems of its own, and I can't do that. (See the thread in #320).

For targets, I prefer to stick with the current behavior unless the same call to vctrs::vec_c() can some how be made to function in all these competing scenarios.

[robitalec]

I think that makes sense, I doesn't seem like an issue {targets} should be managing.
In this case, I ended up just casting the IDates to Dates and could use the iteration on tar_group without any errors.

I am keeping an eye on some {vctrs} issues related to IDates (eg. r-lib/vctrs#1549, r-lib/vctrs#1781) and will update here if this issue is resolved when those are addressed.

Thank you!