Suggestion: Better error message openxlsx2:::assert_workbook

[pteridin]

Background

openxlsx2:::assert_workbook is a function that checks the class of workbook-objects before performing operations on it:

function (x) 
 assert_class(x, c("wbWorkbook", "R6"), all = TRUE)

I happened to write a targets pipeline missing a pipe from one workbook manipulation into the next, resulting in:

Last error: wb must be class wbWorkbook or R6

Which I find to be a little confusing.

Suggestion

I think it would be nice to check for missing wb before asserting class, thus throwing a more precise error message, something along the way of:

check <- function (wb) {
     if(hasArg(wb) == FALSE)
         stop("Workbook `wb` is missing")
     assert_class(wb, c("wbWorkbook", "R6"), all = TRUE)
 }
 

[JanMarvin]

I cannot say that I have a strong opinion on this. But I guess we integrate this into assert_workbook()? (I wasn't aware of hasArg() previously.)

assert <- function(wb) {
  if (!hasArg(wb)) stop("did not found wb")
  print("all good!")
}

assert()
#> Error in assert(): did not found wb
assert(wb = 1)
#> [1] "all good!"
assert(1)
#> [1] "all good!"
wb <- substitute()
assert(wb)
#> Error in assert(wb): did not found wb
wb <- 1
assert(wb)
#> [1] "all good!"

Otherwise we'd have to add a bunch of if (missing(wb)) stop(...) checks into our wrapper pipes and I'd really dislike that.

[JanMarvin]

I have tried to add this into #772

[pteridin]

Thank you 👍

At least I was confused by the error message. Tought that I fiddled with the class of the workbook somewhere along the lines by accident. And the cascades of "Why? Where?" questions started in my head. 😄

[JanMarvin]

Yeah, don't get to addicted to piping ;)

[pteridin]

Too late. 🤣