Bug: Warning in imputation functions is too restrictive

[bundfussr]

What happened?

The following call issues a warning.

  derive_vars_dtm(
    dtc = AESTDTC,
    new_vars_prefix = "AST",
    highest_imputation = "Y",
    time_imputation = "last",
    min_dates = exprs(TEMP_TRTSDTM),
    max_dates = exprs(AENDTM),
    preserve = TRUE
  ) 

It is caused by the following check.

  if (highest_imputation == "Y" & !is.null(min_dates) & date_imputation != "first") {
    warning("If `highest_impuation` = \"Y\" and `min_dates` is specified, `date_imputation` should be set to \"first\".") # nolint
  }

It should be updated to

  if (highest_imputation == "Y" & is.null(min_dates) & date_imputation == "first") {
    warning("If `highest_impuation` = \"Y\" and `date_imputation = \"first\"` is specified, `min_dates` should be specified.") # nolint
  }

Session Information

No response

Reproducible Example

No response

[bundfussr]

@millerg23 , found this bug. We think we should fix it in a hotfix release.

@bms63 , @thomas-neitmann , @zdz2101 , do you agree?

[zdz2101]

I think when I was trying to implement the fix during the release, I tried following the "guidance" here, is there a particular "hierachy" of the function arguments? So for highest_impuation == "Y" , if I'm interpreting correctly, there should be a check of 1:1 correspondence between:

• supplying min_dates and date_imputation == "first"
• supplying max_dates and date_imputation == "last"

Wondering if we need to consider them in separate case-conditions?

[bundfussr]

If highest_imputation == "Y", three conditions should be checked:

• date_imputation %in% c("first", "last")
• if date_imputation == "first" then min_dates should be specified.
• if date_imputation == "last" then max_dates should be specified.

In particular, it is permitted to specify both min_dates and max_dates.