From 35c41175b4cf9d2536e4bb10a25d17f4cb53a576 Mon Sep 17 00:00:00 2001 From: Sebastian Funk Date: Sun, 17 Sep 2023 20:30:01 +0200 Subject: [PATCH] soft deprecate old `dist_spec` interface --- NEWS.md | 2 +- R/dist.R | 2 +- R/estimate_truncation.R | 24 +++++--- R/opts.R | 118 +++++++++++++++++++++++++++++++----- man/delay_opts.Rd | 6 +- man/generation_time_opts.Rd | 24 +++++++- 6 files changed, 147 insertions(+), 29 deletions(-) diff --git a/NEWS.md b/NEWS.md index 5c9368ae7..87671068c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,7 +4,7 @@ This release is in development. For a stable release install 1.3.5 from CRAN. ## Breaking changes -- The external distribution interface has been updated to use the `dist_spec()` function. This comes with a range of benefits, including optimising model fitting when static delays are used (by convolving when first defined vs in stan), easy printing (using `print()`), and easy plotting (using `plot()`). It also makes it possible to use all supported distributions everywhere (i.e, as a generation time or reporting delay). However, this update will break most users code as the interface has changed. See the documentation for `dist_spec()` for more details. By @sbfnk in #363 and reviewed by @seabbs. +- The external distribution interface has been updated to use the `dist_spec()` function. This comes with a range of benefits, including optimising model fitting when static delays are used (by convolving when first defined vs in stan), easy printing (using `print()`), and easy plotting (using `plot()`). It also makes it possible to use all supported distributions everywhere (i.e, as a generation time or reporting delay). However, while for now backwards compatibility has been ensured this update will break most users' code eventually as the interface has changed. See the documentation for `dist_spec()` for more details. By @sbfnk in #363 and reviewed by @seabbs. ## Package diff --git a/R/dist.R b/R/dist.R index 3fe1b2905..7eae43e8e 100644 --- a/R/dist.R +++ b/R/dist.R @@ -817,7 +817,7 @@ sample_approx_dist <- function(cases = NULL, #' tune_inv_gamma <- function(lower = 2, upper = 21) { lifecycle::deprecate_stop( - "1.3.6", "tune_inv_gamma()", + "1.4.0", "tune_inv_gamma()", details = paste0( "As no inverse gamma priors are currently in use and this function has ", "some stability issues it has been deprecated. Please let the package ", diff --git a/R/estimate_truncation.R b/R/estimate_truncation.R index 65f1a40dd..f63fa82e8 100644 --- a/R/estimate_truncation.R +++ b/R/estimate_truncation.R @@ -162,9 +162,11 @@ estimate_truncation <- function(obs, max_truncation, trunc_max = 10, "`max_truncation` and `trunc_max` arguments are both given. ", "Use only `truncation` instead.") } - warning( - "The `trunc_max` argument is deprecated and will be removed in ", - "version 2.0.0. Use `truncation` instead." + deprecate_warn( + "1.4.0", + "estimate_truncation(trunc_max)", + "estimate_truncation(truncation)", + "The argument will be removed completely in version 2.0.0." ) construct_trunc <- TRUE } @@ -174,9 +176,11 @@ estimate_truncation <- function(obs, max_truncation, trunc_max = 10, "`max_truncation` and `truncation` arguments are both given. ", "Use only `truncation` instead.") } - warning( - "The `max_truncation` argument is deprecated and will be removed in ", - "version 2.0.0. Use `truncation` instead." + deprecate_warn( + "1.4.0", + "estimate_truncation(max_truncation)", + "estimate_truncation(truncation)", + "The argument will be removed completely in version 2.0.0." ) trunc_max <- max_truncation construct_trunc <- TRUE @@ -188,9 +192,11 @@ estimate_truncation <- function(obs, max_truncation, trunc_max = 10, "`trunc_dist` and `truncation` arguments are both given. ", "Use only `truncation` instead.") } - warning( - "The `trunc_dist` argument is deprecated and will be removed in ", - "version 2.0.0. Use `truncation` instead." + deprecate_warn( + "1.4.0", + "estimate_truncation(trunc_dist)", + "estimate_truncation(truncation)", + "The argument will be removed completely in version 2.0.0." ) construct_trunc <- TRUE } diff --git a/R/opts.R b/R/opts.R index 1feebb456..1350d145b 100644 --- a/R/opts.R +++ b/R/opts.R @@ -10,6 +10,14 @@ #' using [dist_spec()] or [get_generation_time()]. If no distribution is given #' a fixed generation time of 1 will be assumed. #' +#' @param ... deprecated; use `dist` instead +#' @param disease deprecated; use `dist` instead +#' @param source deprecated; use `dist` instead +#' @param max deprecated; use `dist` instead +#' @param fixed deprecated; use `dist` instead +#' @param prior_weight deprecated; prior weights are now specified as a +#' model option. Use the `weigh_delay_priors` argument of `estimate_infections` +#' instead. #' @return A list summarising the input delay distributions. #' @author Sebastian Funk #' @author Sam Abbott @@ -30,14 +38,65 @@ #' # A generation time sourced from the literature #' dist <- get_generation_time(disease = "SARS-CoV-2", source = "ganyani") #' generation_time_opts(dist) -generation_time_opts <- function(dist = dist_spec(mean = 1)) { +generation_time_opts <- function(dist = dist_spec(mean = 1), ..., + disease, source, max = 15L, fixed = FALSE, + prior_weight) { + deprecated_options_given <- FALSE + dot_options <- list(...) + + ## check consistent options are given + type_options <- (length(dot_options) > 0) + ## distributional parameters + (!missing(disease) && !missing(source)) ## from included distributions + if (type_options > 1) { + stop( + "Generation time can be given either as distributional options ", + "or as disease/source, but not both." + ) + } + if (length(dot_options) > 0) { + if (is(dist, "dist_spec")) { ## dist not specified + dot_options$dist <- "gamma" + } + ## set max + if (!("max" %in% names(dot_options))) { + dot_options$max <- max + } + ## set default of mean=1 for backwards compatibility + if (!("mean" %in% names(dot_options))) { + dot_options$mean <- 1 + } + dot_options$fixed <- fixed + dist <- do.call(dist_spec, dot_options) + deprecated_options_given <- TRUE + } else if (!missing(disease) && !missing(source)) { + dist <- get_generation_time(disease, source, max, fixed) + dist$fixed <- fixed + deprecated_options_given <- TRUE + } if (!is(dist, "dist_spec")) { - stop("The generation time distribution must be given either using a call ", - "to `dist_spec` or `get_generation_time`. ", - "This behaviour has changed from previous versions of `EpiNow2` and ", - "any code using it may need to be updated. For examples and more ", - "information, see the relevant documentation pages using ", - "`?generation_time_opts`") + if (is.list(dist) && length(dot_options) == 0) { + dist <- do.call(dist_spec, dist) + } + deprecated_options_given <- TRUE + } + if (!missing(prior_weight)) { + deprecate_warn( + "1.4.0", "generation_time_opts(prior_weight)", + "estimate_infections(weigh_delay_prior)", + "This argument will be removed in version 2.0.0." + ) + } + if (deprecated_options_given) { + warning( + "The generation time distribution must be given to ", + "`generation_time_opts` using a call to either ", + "`dist_spec` or `get_generation_time`. ", + "This behaviour has changed from previous versions of `EpiNow2` and ", + "any code using it may need to be updated as any other ways of ", + "specifying the generation time are deprecated and will be removed in ", + "version 2.0.0. For examples and more ", + "information, see the relevant documentation pages using ", + "`?generation_time_opts`") } return(dist) } @@ -49,6 +108,8 @@ generation_time_opts <- function(dist = dist_spec(mean = 1)) { #' functions. #' @param dist A delay distribution or series of delay distributions generated #' using [dist_spec()]. Default is an empty call to [dist_spec()], i.e. no delay +#' @param ... deprecated; use `dist` instead +#' @param fixed deprecated; use `dist` instead #' @return A list summarising the input delay distributions. #' @author Sam Abbott #' @author Sebastian Funk @@ -68,16 +129,36 @@ generation_time_opts <- function(dist = dist_spec(mean = 1)) { #' #' # Multiple delays (in this case twice the same) #' delay_opts(delay + delay) -delay_opts <- function(dist = dist_spec()) { - if (!is(dist, "dist_spec")) { - stop( +delay_opts <- function(dist = dist_spec(), ..., fixed = FALSE) { + dot_options <- list(...) + if (!is(dist, "dist_spec")) { ## could be old syntax + if (is.list(dist)) { + ## combine lists if more than one given + dot_options <- c(list(dist), dot_options) + dist <- lapply(dot_options, do.call, what = dist_spec) + if (length(dist) > 1) { + for (i in seq(2, length(dist))) { + dist[[1]] <- dist[[1]] + dist[[i]] + } + } + dist <- dist[[1]] + } else { + stop("`dist` should be given as result of a call to `dist_spec`.") + } + warning( "Delay distributions must be of given either using a call to ", "`dist_spec` or one of the `get_...` functions such as ", - "`get_incubation_period`. This behaviour has changed from previous ", - "versions of `EpiNow2` and any code using it may need to be updated. ", - "For examples and more information, see the relevant documentation ", - "pages using `?delay_opts`." + "`get_incubation_period`. ", + "This behaviour has changed from previous versions of `EpiNow2` and ", + "any code using it may need to be updated as any other ways of ", + "specifying delays are deprecated and will be removed in ", + "version 2.0.0. For examples and more ", + "information, see the relevant documentation pages using ", + "`?delay_opts`." ) + } else if (length(dot_options) > 0) { + ## can be removed once dot options are hard deprecated + stop("Unknown named arguments passed to `delay_opts`" ) } return(dist) } @@ -106,11 +187,16 @@ delay_opts <- function(dist = dist_spec()) { #' trunc_opts(dist = dist_spec(mean = 3, sd = 2, max = 10)) trunc_opts <- function(dist = dist_spec()) { if (!is(dist, "dist_spec")) { - stop( + if (is.list(dist)) { + dist <- do.call(dist_spec, dist) + } + warning( "Truncation distributions must be of given either using a call to ", "`dist_spec` or one of the `get_...` functions. ", "This behaviour has changed from previous versions of `EpiNow2` and ", - "any code using it may need to be updated. For examples and more ", + "any code using it may need to be updated as any other ways of ", + "specifying delays are deprecated and will be removed in ", + "version 2.0.0. For examples and more ", "information, see the relevant documentation pages using ", "`?trunc_opts`" ) diff --git a/man/delay_opts.Rd b/man/delay_opts.Rd index 114ab92f1..989d6d2dc 100644 --- a/man/delay_opts.Rd +++ b/man/delay_opts.Rd @@ -4,11 +4,15 @@ \alias{delay_opts} \title{Delay Distribution Options} \usage{ -delay_opts(dist = dist_spec()) +delay_opts(dist = dist_spec(), ..., fixed = FALSE) } \arguments{ \item{dist}{A delay distribution or series of delay distributions generated using \code{\link[=dist_spec]{dist_spec()}}. Default is an empty call to \code{\link[=dist_spec]{dist_spec()}}, i.e. no delay} + +\item{...}{deprecated; use \code{dist} instead} + +\item{fixed}{deprecated; use \code{dist} instead} } \value{ A list summarising the input delay distributions. diff --git a/man/generation_time_opts.Rd b/man/generation_time_opts.Rd index 64a90997b..2674b201e 100644 --- a/man/generation_time_opts.Rd +++ b/man/generation_time_opts.Rd @@ -4,12 +4,34 @@ \alias{generation_time_opts} \title{Generation Time Distribution Options} \usage{ -generation_time_opts(dist = dist_spec(mean = 1)) +generation_time_opts( + dist = dist_spec(mean = 1), + ..., + disease, + source, + max = 15L, + fixed = FALSE, + prior_weight +) } \arguments{ \item{dist}{A delay distribution or series of delay distributions generated using \code{\link[=dist_spec]{dist_spec()}} or \code{\link[=get_generation_time]{get_generation_time()}}. If no distribution is given a fixed generation time of 1 will be assumed.} + +\item{...}{deprecated; use \code{dist} instead} + +\item{disease}{deprecated; use \code{dist} instead} + +\item{source}{deprecated; use \code{dist} instead} + +\item{max}{deprecated; use \code{dist} instead} + +\item{fixed}{deprecated; use \code{dist} instead} + +\item{prior_weight}{deprecated; prior weights are now specified as a +model option. Use the \code{weigh_delay_priors} argument of \code{estimate_infections} +instead.} } \value{ A list summarising the input delay distributions.