From 0d1447e354d431e86bcb9c027a39e073f0f74cc0 Mon Sep 17 00:00:00 2001 From: Shreya Rao Date: Thu, 14 Nov 2024 10:46:53 +1100 Subject: [PATCH] handling deprecated arguments --- DESCRIPTION | 4 ++- NAMESPACE | 2 ++ NEWS.txt | 24 ++++++++++++- R/spicy.R | 86 ++++++++++++++++++++++++++++++---------------- R/utilities.R | 71 ++++++++++++++++++++++++++++++++++++++ man/getPairwise.Rd | 27 +++++++++------ man/spicy.Rd | 31 ++++++++++------- 7 files changed, 189 insertions(+), 56 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 9440722..9ff0661 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -49,7 +49,9 @@ Imports: ggthemes, ggh4x, coxme, - ggnewscale + ggnewscale, + lifecycle, + simpleSeg Suggests: SpatialDatasets, BiocStyle, diff --git a/NAMESPACE b/NAMESPACE index 988aff8..b866fd5 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -66,6 +66,8 @@ importFrom(grDevices,colors) importFrom(grid,gpar) importFrom(grid,grobTree) importFrom(grid,polygonGrob) +importFrom(lifecycle,deprecate_soft) +importFrom(lifecycle,deprecate_warn) importFrom(lmerTest,lmer) importFrom(magrittr,"%>%") importFrom(methods,is) diff --git a/NEWS.txt b/NEWS.txt index adde84b..1fab5db 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -22,4 +22,26 @@ BUG FIXES SIGNIFICANT USER-VISIBLE CHANGES o Vignette has been updated with new dataset showcasing more functionality - of spicyR. \ No newline at end of file + of spicyR. + +DEPRECATED ARGUMENTS + + o `nCores` argument deprecated for both getPairwise() and spicy(), replaced + with `cores`. + + o `BPPARAM` argument deprecated for both getPairwise() and spicy(), replaced + with `cores`. + + o `imageIDCol` argument deprecated for both getPairwise() and spicy(), replaced + with `imageID`. + + o `cellTypeCol` argument deprecated for both getPairwise() and spicy(), replaced + with `cellType`. + + o `spatialCoordCols` argument deprecated for both getPairwise() and spicy(), replaced + with `spatialCoords`. + +* Deprecation warnings have been added to guide users to the new argument names. + + + diff --git a/R/spicy.R b/R/spicy.R index 571984d..1f66b24 100644 --- a/R/spicy.R +++ b/R/spicy.R @@ -7,25 +7,25 @@ #' a data frame. #' @param covariates Vector of covariate names that should be included in the #' mixed effects model as fixed effects. -#' @param from vector of cell types which you would like to compare to the to vector. -#' @param to vector of cell types which you would like to compare to the from vector. #' @param imageID The name of the imageID column if using a SingleCellExperiment or SpatialExperiment. #' @param cellType The name of the cellType column if using a SingleCellExperiment or SpatialExperiment. #' @param spatialCoords The names of the spatialCoords column if using a SingleCellExperiment. #' @param Rs A vector of the radii that the measures of association should be calculated over. #' @param sigma A numeric variable used for scaling when fitting inhomogenous L-curves. +#' @param from vector of cell types which you would like to compare to the to vector. +#' @param to vector of cell types which you would like to compare to the from vector. +#' @param alternateResult A pairwise association statistic between each combination of celltypes in +#' each image. +#' @param cores Number of cores to use for parallel processing or a BiocParallel MulticoreParam or SerialParam object. #' @param minLambda Minimum value density for scaling when fitting inhomogeneous L-curves. #' @param weights logical indicating whether to include weights based on cell counts. #' @param weightsByPair logical indicating whether weights should be calculated for each cell type #' pair. #' @param weightFactor numeric that controls the convexity of the weight function. -#' @param alternateResult A pairwise association statistic between each combination of celltypes in -#' each image. #' @param window Should the window around the regions be 'square', 'convex' or 'concave'. #' @param window.length A tuning parameter for controlling the level of concavity when estimating concave windows. #' @param edgeCorrect A logical indicating whether to perform edge correction. #' @param includeZeroCells A logical indicating whether to include cells with zero counts in the pairwise association calculation. -#' @param nCores Number of cores to use for parallel processing. #' @param verbose logical indicating whether to output messages. #' @param ... Other options #' @return Data frame of p-values. @@ -58,31 +58,39 @@ #' @importFrom scam scam #' @importFrom rlang .data #' @importFrom tibble column_to_rownames -#' @importFrom BiocParallel MulticoreParam +#' @importFrom lifecycle deprecate_soft spicy <- function(cells, condition, subject = NULL, covariates = NULL, - from = NULL, - to = NULL, imageID = "imageID", cellType = "cellType", spatialCoords = c("x", "y"), Rs = NULL, sigma = NULL, + from = NULL, + to = NULL, + alternateResult = NULL, + cores = 1, minLambda = 0.05, weights = TRUE, weightsByPair = FALSE, weightFactor = 1, - alternateResult = NULL, window = "convex", window.length = NULL, edgeCorrect = TRUE, includeZeroCells = FALSE, - nCores = 1, + BPPARAM = BiocParallel::SerialParam(), + imageIDCol = imageID, + cellTypeCol = cellType, + spatialCoordCols = spatialCoords, + nCores = cores, verbose = FALSE, ...) { - BPPARAM <- BiocParallel::MulticoreParam(workers = nCores) + + user_args = as.list(match.call())[-1] + user_vals = lapply(user_args, eval) + argumentChecks("spicy", user_vals) if (is(cells, "SummarizedExperiment") || is(cells, "data.frame")) { cells <- .format_data( @@ -159,16 +167,16 @@ spicy <- function(cells, if (is.null(alternateResult)) { pairwiseAssoc <- getPairwise(cells, - Rs = Rs, - sigma = sigma, - window = window, - window.length = window.length, - minLambda = minLambda, - from = from, - to = to, - edgeCorrect = edgeCorrect, - includeZeroCells = includeZeroCells, - nCores = nCores + Rs = Rs, + sigma = sigma, + from = from, + to = to, + cores = cores, + minLambda = minLambda, + window = window, + window.length = window.length, + edgeCorrect = edgeCorrect, + includeZeroCells = includeZeroCells ) pairwiseAssoc <- as.data.frame(pairwiseAssoc) pairwiseAssoc <- pairwiseAssoc[labels] @@ -400,21 +408,21 @@ cleanMEM <- function(mixed.lmer, BPPARAM) { #' @param cells A SummarizedExperiment that contains at least the #' variables x and y, giving the location coordinates of each cell, and #' cellType. -#' @param from The 'from' cellType for generating the L curve. -#' @param to The 'to' cellType for generating the L curve. #' @param imageID #' The name of the imageID column if using a SingleCellExperiment or SpatialExperiment. #' @param cellType The name of the cellType column if using a SingleCellExperiment or SpatialExperiment. #' @param spatialCoords The names of the spatialCoords column if using a SingleCellExperiment. #' @param Rs A vector of the radii that the measures of association should be calculated over. #' @param sigma A numeric variable used for scaling when fitting inhomogenous L-curves. +#' @param from The 'from' cellType for generating the L curve. +#' @param to The 'to' cellType for generating the L curve. +#' @param cores Number of cores to use for parallel processing or a BiocParallel MulticoreParam or SerialParam object. #' @param minLambda Minimum value density for scaling when fitting inhomogeneous L-curves. #' @param window Should the window around the regions be 'square', 'convex' or 'concave'. #' @param window.length A tuning parameter for controlling the level of concavity when estimating concave windows. #' @param edgeCorrect A logical indicating whether to perform edge correction. #' @param includeZeroCells A logical indicating whether to include cells with zero counts in the pairwise association #' calculation. -#' @param nCores Number of cores to use for parallel processing. #' @return Statistic from pairwise L-curve of a single image. #' @examples #' data("diabetesData") @@ -428,21 +436,39 @@ cleanMEM <- function(mixed.lmer, BPPARAM) { #' @importFrom BiocParallel MulticoreParam getPairwise <- function( cells, - from = NULL, - to = NULL, imageID = "imageID", cellType = "cellType", spatialCoords = c("x", "y"), - Rs = c(20, 50, 100), + Rs = NULL, sigma = NULL, + from = NULL, + to = NULL, + cores = 1, minLambda = 0.05, window = "convex", window.length = NULL, edgeCorrect = TRUE, - includeZeroCells = TRUE, - nCores = 1 + includeZeroCells = FALSE, + BPPARAM = BiocParallel::SerialParam(), + imageIDCol = imageID, + cellTypeCol = cellType, + spatialCoordCols = spatialCoords, + nCores = cores ) { - BPPARAM <- BiocParallel::MulticoreParam(workers = nCores) + + user_args = as.list(match.call())[-1] + + tryCatch({ + user_vals = lapply(user_args, eval) + argumentChecks("getPairwise", user_vals) + }, error = function(e) { + if (grepl("object 'cells' not found", e$message)) { + message("Skipping argument checks as `getPairwise()` is being called within `spicy()`") + } else { + stop(e) + } + }) + if (is(cells, "SummarizedExperiment")) { cells <- .format_data( diff --git a/R/utilities.R b/R/utilities.R index d8b7c0a..ecdc446 100644 --- a/R/utilities.R +++ b/R/utilities.R @@ -167,3 +167,74 @@ getImagePheno <- function(x, # if (expand) rownames(x) <- x$imageID x } + +#' A function to handle validity of argumemts/check for deprecated arguments +#' +#' @importFrom lifecycle deprecate_warn +#' @importFrom methods is +#' @noRd +argumentChecks = function(function_name, user_vals) { + + rlang::local_options(lifecycle_verbosity = "warning") + + # handle deprecated arguments + handle_deprecated = function(old_arg, new_arg, user_vals) { + if (old_arg %in% names(user_vals) && !(new_arg %in% names(user_vals))) { + # warning(paste0("'", old_arg, "' was deprecated in 1.18.0. Please use '", new_arg, "' instead.\n")) + deprecate_warn("1.18.0", paste0(function_name, "(", old_arg, ")"), paste0(function_name, "(", new_arg, ")")) + assign(new_arg, user_vals[[old_arg]], envir = sys.frame(sys.parent(1))) + } + } + + handle_deprecated("imageIDCol", "imageID", user_vals) + handle_deprecated("cellTypeCol", "cellType", user_vals) + handle_deprecated("spatialCoordCols", "spatialCoords", user_vals) + handle_deprecated("nCores", "cores", user_vals) + handle_deprecated("BPPARAM", "cores", user_vals) + + # enforce mutually exclusive arguments + check_exclusive = function(arg_set, user_vals) { + provided_args = intersect(arg_set, names(user_vals)) + if (length(provided_args) > 1) { + stop(paste("Please specify only one of", paste(shQuote(arg_set), collapse = ", "), "\n")) + } + } + + check_exclusive(c("cellTypeCol", "cellType"), user_vals) + check_exclusive(c("imageIDCol", "imageID"), user_vals) + check_exclusive(c("spatialCoordCols", "spatialCoords"), user_vals) + check_exclusive(c("cores", "nCores", "BPPARAM"), user_vals) + + # validity checks for cores/nCores/BPPARAM + if ("nCores" %in% names(user_vals)) { + # warning("'nCores' was deprecated in 1.18.0. Please use 'cores' instead.\n") + deprecate_warn("1.18.0", paste0(function_name, "(nCores)"), paste0(function_name, "(cores)")) + + if (is(user_vals$nCores, "numeric")) { + assign("cores", simpleSeg:::generateBPParam(cores = user_vals$nCores), envir = parent.frame()) + } else if (is(user_vals$nCores, "MulticoreParam") || is(user_vals$nCores, "SerialParam")) { + assign("cores", user_vals$nCores, envir = sys.frame(sys.parent(1))) + } else { + stop("'nCores' must be either a numeric value, or a MulticoreParam or SerialParam object.\n") + } + + } else if ("BPPARAM" %in% names(user_vals)) { + # warning("'BPPARAM' was deprecated in 1.18.0. Please use 'cores' instead.\n") + deprecate_warn("1.18.0", paste0(function_name, "(BPPARAM)"), paste0(function_name, "(cores)")) + + if (is(user_vals$BPPARAM, "MulticoreParam") || is(user_vals$BPPARAM, "SerialParam")) { + assign("cores", user_vals$BPPARAM, envir = sys.frame(sys.parent(1))) + } else { + stop("'BBPARAM' must be a MulticoreParam or SerialParam object.") + } + + } else if ("cores" %in% names(user_vals)) { + if (is(user_vals$cores, "numeric")) { + assign("cores", simpleSeg:::generateBPParam(cores = user_vals$cores)) + } else if (is(user_vals$cores, "MulticoreParam") || is(user_vals$cores, "SerialParam")) { + assign("cores", user_vals$cores, sys.frame(sys.parent(1))) + } else { + stop("'cores' must be either a numeric value, or a MulticoreParam or SerialParam object.\n") + } + } +} diff --git a/man/getPairwise.Rd b/man/getPairwise.Rd index af451cf..cf0e7eb 100644 --- a/man/getPairwise.Rd +++ b/man/getPairwise.Rd @@ -6,19 +6,24 @@ \usage{ getPairwise( cells, - from = NULL, - to = NULL, imageID = "imageID", cellType = "cellType", spatialCoords = c("x", "y"), - Rs = c(20, 50, 100), + Rs = NULL, sigma = NULL, + from = NULL, + to = NULL, + cores = 1, minLambda = 0.05, window = "convex", window.length = NULL, edgeCorrect = TRUE, - includeZeroCells = TRUE, - nCores = 1 + includeZeroCells = FALSE, + BPPARAM = BiocParallel::SerialParam(), + imageIDCol = imageID, + cellTypeCol = cellType, + spatialCoordCols = spatialCoords, + nCores = cores ) } \arguments{ @@ -26,10 +31,6 @@ getPairwise( variables x and y, giving the location coordinates of each cell, and cellType.} -\item{from}{The 'from' cellType for generating the L curve.} - -\item{to}{The 'to' cellType for generating the L curve.} - \item{imageID}{The name of the imageID column if using a SingleCellExperiment or SpatialExperiment.} \item{cellType}{The name of the cellType column if using a SingleCellExperiment or SpatialExperiment.} @@ -40,6 +41,12 @@ cellType.} \item{sigma}{A numeric variable used for scaling when fitting inhomogenous L-curves.} +\item{from}{The 'from' cellType for generating the L curve.} + +\item{to}{The 'to' cellType for generating the L curve.} + +\item{cores}{Number of cores to use for parallel processing or a BiocParallel MulticoreParam or SerialParam object.} + \item{minLambda}{Minimum value density for scaling when fitting inhomogeneous L-curves.} \item{window}{Should the window around the regions be 'square', 'convex' or 'concave'.} @@ -50,8 +57,6 @@ cellType.} \item{includeZeroCells}{A logical indicating whether to include cells with zero counts in the pairwise association calculation.} - -\item{nCores}{Number of cores to use for parallel processing.} } \value{ Statistic from pairwise L-curve of a single image. diff --git a/man/spicy.Rd b/man/spicy.Rd index 1773db6..93496a0 100644 --- a/man/spicy.Rd +++ b/man/spicy.Rd @@ -13,23 +13,28 @@ spicy( condition, subject = NULL, covariates = NULL, - from = NULL, - to = NULL, imageID = "imageID", cellType = "cellType", spatialCoords = c("x", "y"), Rs = NULL, sigma = NULL, + from = NULL, + to = NULL, + alternateResult = NULL, + cores = 1, minLambda = 0.05, weights = TRUE, weightsByPair = FALSE, weightFactor = 1, - alternateResult = NULL, window = "convex", window.length = NULL, edgeCorrect = TRUE, includeZeroCells = FALSE, - nCores = 1, + BPPARAM = BiocParallel::SerialParam(), + imageIDCol = imageID, + cellTypeCol = cellType, + spatialCoordCols = spatialCoords, + nCores = cores, verbose = FALSE, ... ) @@ -46,10 +51,6 @@ a data frame.} \item{covariates}{Vector of covariate names that should be included in the mixed effects model as fixed effects.} -\item{from}{vector of cell types which you would like to compare to the to vector.} - -\item{to}{vector of cell types which you would like to compare to the from vector.} - \item{imageID}{The name of the imageID column if using a SingleCellExperiment or SpatialExperiment.} \item{cellType}{The name of the cellType column if using a SingleCellExperiment or SpatialExperiment.} @@ -60,6 +61,15 @@ mixed effects model as fixed effects.} \item{sigma}{A numeric variable used for scaling when fitting inhomogenous L-curves.} +\item{from}{vector of cell types which you would like to compare to the to vector.} + +\item{to}{vector of cell types which you would like to compare to the from vector.} + +\item{alternateResult}{A pairwise association statistic between each combination of celltypes in +each image.} + +\item{cores}{Number of cores to use for parallel processing or a BiocParallel MulticoreParam or SerialParam object.} + \item{minLambda}{Minimum value density for scaling when fitting inhomogeneous L-curves.} \item{weights}{logical indicating whether to include weights based on cell counts.} @@ -69,9 +79,6 @@ pair.} \item{weightFactor}{numeric that controls the convexity of the weight function.} -\item{alternateResult}{A pairwise association statistic between each combination of celltypes in -each image.} - \item{window}{Should the window around the regions be 'square', 'convex' or 'concave'.} \item{window.length}{A tuning parameter for controlling the level of concavity when estimating concave windows.} @@ -80,8 +87,6 @@ each image.} \item{includeZeroCells}{A logical indicating whether to include cells with zero counts in the pairwise association calculation.} -\item{nCores}{Number of cores to use for parallel processing.} - \item{verbose}{logical indicating whether to output messages.} \item{...}{Other options}