From 7a944a74a938185d3400ff66d613ec9d55825caa Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 4 Oct 2023 12:26:57 +0200 Subject: [PATCH 01/22] define within method for qenv class --- DESCRIPTION | 1 + NAMESPACE | 1 + R/qenv-within.R | 28 ++++++++++++++++++++++++++++ 3 files changed, 30 insertions(+) create mode 100644 R/qenv-within.R diff --git a/DESCRIPTION b/DESCRIPTION index e6cf8a5eb..efa394a84 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -59,5 +59,6 @@ Collate: 'qenv-get_warnings.R' 'qenv-join.R' 'qenv-show.R' + 'qenv-within.R' 'teal.code-package.R' 'utils.R' diff --git a/NAMESPACE b/NAMESPACE index aa2789359..d7211a97c 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,6 +1,7 @@ # Generated by roxygen2: do not edit by hand S3method("[[",qenv.error) +S3method(within,qenv) export(concat) export(dev_suppress) export(eval_code) diff --git a/R/qenv-within.R b/R/qenv-within.R new file mode 100644 index 000000000..ba162b036 --- /dev/null +++ b/R/qenv-within.R @@ -0,0 +1,28 @@ +#' @export +within.qenv <- function(data, expr, ...) { + expr <- substitute(expr) + extras <- list(...) + + # Get parent call to use in error messages. + parent_call <- deparse1(match.call(definition = sys.function(1L), call = sys.call(1L))) + + if (is.symbol(expr)) { + warning( + "In ", parent_call, " : Symbol passed to \"expr\" will be evaluated within qenv as is. ", "\n", + "This function is intended for literal expressions, for variables containing code use `eval_code` instead.", + call. = FALSE + ) + } + + # Add braces for consistency. + if (!identical(as.list(expr)[[1L]], as.symbol("{"))) { + expr <- call("{", expr) + } + # Drop strings from compound expressions. + calls <- Filter(Negate(is.character), as.list(expr)[-1]) + + # Inject extra values into expressions. + calls <- lapply(calls, function(x) do.call(substitute, list(x, env = extras))) + + eval_code(object = data, code = as.expression(calls)) +} From 0a1beaacd3caba9b4a682ce69171388c332ed1da Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 4 Oct 2023 12:27:36 +0200 Subject: [PATCH 02/22] add unit tests --- tests/testthat/test-qenv-within.R | 173 ++++++++++++++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 tests/testthat/test-qenv-within.R diff --git a/tests/testthat/test-qenv-within.R b/tests/testthat/test-qenv-within.R new file mode 100644 index 000000000..c21cfb3c6 --- /dev/null +++ b/tests/testthat/test-qenv-within.R @@ -0,0 +1,173 @@ +# styler: off +# nolint start + +# evaluation ---- +## code acceptance ---- +testthat::test_that("simple expressions are evaluated", { + q <- new_qenv() + testthat::expect_no_error(within(q, 1 + 1)) +}) + +testthat::test_that("compound expressions are evaluated", { + q <- new_qenv() + testthat::expect_no_error( + within(q, { + 1 + 1 + }) + ) + testthat::expect_no_error( + within(q, { + 1 + 1 + 2 + 2 + }) + ) + testthat::expect_no_error( + within(q, { + 1 + 1; 2 + 2 + }) + ) + testthat::expect_no_error( + within(q, { + 1 + + 1 + }) + ) +}) + +testthat::test_that("characters in compound expressions are omitted", { + q <- new_qenv() + qq <- within(q, { + i <- iris + "1 + 1" + m <- mtcars + }) + testthat::expect_identical(get_code(qq), c("i <- iris", "m <- mtcars")) +}) + +testthat::test_that("character-only compound expressions are ignored", { + q <- new_qenv() + qq <- within(q, {"1 + 1"}) + testthat::expect_equal(q, qq) +}) + +testthat::test_that("symbol passed to `expr` raises warning", { + q <- new_qenv() + q <- within(q, i <- iris) + testthat::expect_warning( + qq <- within(q, i), + "Symbol passed to \"expr\"" + ) + testthat::expect_identical(get_code(qq), c("i <- iris", "i")) +}) + + +## code identity ---- +testthat::test_that("differently formulated expressions yield the same code", { + q <- new_qenv() + q <- within(q, 1 + 1) + q <- within(q, {1 + 1}) + q <- within(q, { + 1 + 1 + }) + q <- within(q, { + 1 + + 1 + }) + all_code <- get_code(q) + testthat::expect_identical( + all_code, + rep("1 + 1", 4L) + ) + + q <- new_qenv() + q <- within(q, {1 + 1; 2 + 2}) + q <- within(q, { + 1 + 1; 2 + 2 + }) + q <- within(q, { + 1 + 1 + 2 + 2 + }) + q <- within(q, { + 1 + 1; + 2 + 2 + }) + all_code <- get_code(q) + all_code_pairs <- lapply(seq_len(4L), function(x) all_code[((x - 1L) * 2L) + 1:2]) + testthat::expect_identical( + all_code, + rep(c("1 + 1", "2 + 2"), 4L) + ) +}) + + +## injecting values ---- +testthat::test_that("external values can be injected into expressions through `...`", { + q <- new_qenv() + + q <- within(q, { + i <- subset(iris, Species == "setosa") + }) + + q <- within(q, { + ii <- subset(iris, Species == species) + }, + species = "virginica") + + external_value <- "versicolor" + q <- within(q, { + iii <- subset(iris, Species == species) + }, + species = external_value) + + testthat::expect_identical( + get_code(q), + c( + "i <- subset(iris, Species == \"setosa\")", + "ii <- subset(iris, Species == \"virginica\")", + "iii <- subset(iris, Species == \"versicolor\")" + ) + ) + + q <- new_qenv() + species <- "setosa" + qq <- within(q, { + i <- subset(iris, Species == species) + }) + testthat::expect_s3_class(qq, "qenv.error") + + qq <- within(q, { + i <- subset(iris, Species == species) + }, + species = species) + testthat::expect_s4_class(qq, "qenv") +}) + + +# return value ---- +testthat::test_that("within.qenv renturns a deep copy of `data`", { + q <- new_qenv() + q <- within(new_qenv(), i <- iris) + qq <- within(q, {}) + testthat::expect_equal(q, qq) + + q <- new_qenv() + q <- within(q, i <- iris) + qq <- within(q, m <- mtcars) + testthat::expect_failure( + testthat::expect_equal(q, qq) + ) +}) + +testthat::test_that("within.qenv renturns qenv.error even if evaluation raises error", { + q <- new_qenv() + q <- within(q, i <- iris) + qq <- within(q, stop("right there")) + testthat::expect_true( + exists("qq", inherits = FALSE) + ) + testthat::expect_s3_class(qq, "qenv.error") +}) + +# nolint end +# styler: on From 34378c769e29a7cf270a720b9bd862600a429490 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 4 Oct 2023 12:56:48 +0200 Subject: [PATCH 03/22] add documentation --- R/qenv-within.R | 46 +++++++++++++++++++++++++++++++++++++++++ man/within.Rd | 55 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) create mode 100644 man/within.Rd diff --git a/R/qenv-within.R b/R/qenv-within.R index ba162b036..786145308 100644 --- a/R/qenv-within.R +++ b/R/qenv-within.R @@ -1,4 +1,50 @@ +#' Evaluate expression in `qenv` object. +#' +#' Convenience function for evaluating inline code inside the environment of a `qenv`. +#' +#' This is a wrapper for `eval_code` that provides a simplified way of passing code for evaluation. +#' It accepts only inline expressions (both simple and compound) and allows for injecting values into `expr` +#' through the `...` argument: as `name:value` pairs are passed to `...`, +#' `name` in `expr` will be replaced with `value`. +#' +#' +#' @param data `qenv` object +#' @param expr `expression` to evaluate +#' @param ... `name:value` pairs to inject values into `expr` +#' +#' @return +#' Returns a `qenv` object with `expr` evaluated. If evaluation raises an error, a `qenv.error` is returned. +#' +#' @seealso [`eval_code`], [`base::within`] +#' #' @export +#' +#' @rdname within +#' +#' @examples +#' +#' q <- new_qenv() +#' +#' # execute code +#' q <- within(q, { +#' i <- iris +#' }) +#' q <- within(q, { +#' m <- mtcars +#' f <- faithful +#' }) +#' q +#' get_code(q) +#' +#' # inject values into code +#' q <- new_qenv() +#' q <- within(q, i <- iris) +#' within(q, print(dim(subset(i, Species == "virginica")))) +#' within(q, print(dim(subset(i, Species == species)))) # fails +#' within(q, print(dim(subset(i, Species == species))), species = "versicolor") +#' species_external <- "versicolor" +#' within(q, print(dim(subset(i, Species == species))), species = species_external) +#' within.qenv <- function(data, expr, ...) { expr <- substitute(expr) extras <- list(...) diff --git a/man/within.Rd b/man/within.Rd new file mode 100644 index 000000000..ae4a09b8d --- /dev/null +++ b/man/within.Rd @@ -0,0 +1,55 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/qenv-within.R +\name{within.qenv} +\alias{within.qenv} +\title{Evaluate expression in \code{qenv} object.} +\usage{ +\method{within}{qenv}(data, expr, ...) +} +\arguments{ +\item{data}{\code{qenv} object} + +\item{expr}{\code{expression} to evaluate} + +\item{...}{\code{name:value} pairs to inject values into \code{expr}} +} +\value{ +Returns a \code{qenv} object with \code{expr} evaluated. If evaluation raises an error, a \code{qenv.error} is returned. +} +\description{ +Convenience function for evaluating inline code inside the environment of a \code{qenv}. +} +\details{ +This is a wrapper for \code{eval_code} that provides a simplified way of passing code for evaluation. +It accepts only inline expressions (both simple and compound) and allows for injecting values into \code{expr} +through the \code{...} argument: as \code{name:value} pairs are passed to \code{...}, +\code{name} in \code{expr} will be replaced with \code{value}. +} +\examples{ + +q <- new_qenv() + +# execute code +q <- within(q, { + i <- iris +}) +q <- within(q, { + m <- mtcars + f <- faithful +}) +q +get_code(q) + +# inject values into code +q <- new_qenv() +q <- within(q, i <- iris) +within(q, print(dim(subset(i, Species == "virginica")))) +within(q, print(dim(subset(i, Species == species)))) # fails +within(q, print(dim(subset(i, Species == species))), species = "versicolor") +species_external <- "versicolor" +within(q, print(dim(subset(i, Species == species))), species = species_external) + +} +\seealso{ +\code{\link{eval_code}}, \code{\link[base:with]{base::within}} +} From 23a21afe2ea3a3ef3b75759240cf08c770e8c61e Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 4 Oct 2023 11:15:13 +0000 Subject: [PATCH 04/22] [skip actions] Restyle files --- R/qenv-within.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/qenv-within.R b/R/qenv-within.R index 786145308..6d14a15a6 100644 --- a/R/qenv-within.R +++ b/R/qenv-within.R @@ -40,7 +40,7 @@ #' q <- new_qenv() #' q <- within(q, i <- iris) #' within(q, print(dim(subset(i, Species == "virginica")))) -#' within(q, print(dim(subset(i, Species == species)))) # fails +#' within(q, print(dim(subset(i, Species == species)))) # fails #' within(q, print(dim(subset(i, Species == species))), species = "versicolor") #' species_external <- "versicolor" #' within(q, print(dim(subset(i, Species == species))), species = species_external) From 3a23bb1fa9a2315d451184da7dd7d847a1d3951a Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 4 Oct 2023 13:27:17 +0200 Subject: [PATCH 05/22] trigger From bfc738ff53d4efa0f9761e34f3afcc28eaa5344c Mon Sep 17 00:00:00 2001 From: "27856297+dependabot-preview[bot]@users.noreply.github.com" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 4 Oct 2023 11:29:29 +0000 Subject: [PATCH 06/22] [skip actions] Roxygen Man Pages Auto Update --- man/within.Rd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/within.Rd b/man/within.Rd index ae4a09b8d..5cab9155b 100644 --- a/man/within.Rd +++ b/man/within.Rd @@ -44,7 +44,7 @@ get_code(q) q <- new_qenv() q <- within(q, i <- iris) within(q, print(dim(subset(i, Species == "virginica")))) -within(q, print(dim(subset(i, Species == species)))) # fails +within(q, print(dim(subset(i, Species == species)))) # fails within(q, print(dim(subset(i, Species == species))), species = "versicolor") species_external <- "versicolor" within(q, print(dim(subset(i, Species == species))), species = species_external) From cd259afb8bc0ac9f2ff684e8f8ae6c6de8d84a3c Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 4 Oct 2023 13:32:37 +0200 Subject: [PATCH 07/22] trigger From ff3b6b758b72166a710c714e97f68b0b87b98930 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 4 Oct 2023 13:35:01 +0200 Subject: [PATCH 08/22] amend NEWS --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index 66b29f0c3..94b170a2e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,6 +7,7 @@ * Updated usage and installation instructions in `README`. * Updated phrasing of the `qenv` vignette. * Specified minimal version of package dependencies. +* Added `within` method for `qenv` for cenvenient passing of inline code to `eval_code`. # teal.code 0.4.0 From e6823a2a3f3225a95ea161055775a64b03c04d0c Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 4 Oct 2023 13:37:34 +0200 Subject: [PATCH 09/22] fix typo --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 94b170a2e..4bd5f8211 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,7 +7,7 @@ * Updated usage and installation instructions in `README`. * Updated phrasing of the `qenv` vignette. * Specified minimal version of package dependencies. -* Added `within` method for `qenv` for cenvenient passing of inline code to `eval_code`. +* Added `within` method for `qenv` for convenient passing of inline code to `eval_code`. # teal.code 0.4.0 From 1e77c7166df70ed54b965c198cbdeace07270a07 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 4 Oct 2023 13:47:52 +0200 Subject: [PATCH 10/22] update pkgdown --- _pkgdown.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/_pkgdown.yml b/_pkgdown.yml index 19958db9c..46dfe638a 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -33,3 +33,4 @@ reference: - join - new_qenv - show,qenv-method + - within.qenv From beefbc62f3692cb08e04f393bbcec3a9a511ff06 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 4 Oct 2023 13:48:08 +0200 Subject: [PATCH 11/22] minor edit in vignette --- vignettes/qenv.Rmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vignettes/qenv.Rmd b/vignettes/qenv.Rmd index bb2d985c2..583939675 100644 --- a/vignettes/qenv.Rmd +++ b/vignettes/qenv.Rmd @@ -1,5 +1,5 @@ --- -title: "`qenv`" +title: "qenv" author: "NEST coreDev" output: rmarkdown::html_vignette vignette: > From fef2f25561de5a6e745d1891301dbfa46116bbeb Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 4 Oct 2023 14:37:33 +0200 Subject: [PATCH 12/22] update vignette --- vignettes/qenv.Rmd | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/vignettes/qenv.Rmd b/vignettes/qenv.Rmd index 583939675..2af944447 100644 --- a/vignettes/qenv.Rmd +++ b/vignettes/qenv.Rmd @@ -46,6 +46,36 @@ print(my_qenv) print(q2) ``` + +In some cases one may want to substitute some elements of the code before evaluation. +Consider a case when a subset of `iris` defined by an input value. +```{r} +q <- new_qenv() +q <- eval_code(q, quote(i <- subset(iris, Species == "setosa"))) +q <- eval_code(q, substitute(ii <- subset(iris, Species == species), env = list(species = "versicolor"))) +input_value <- "virginica" +q <- eval_code(q, substitute(iii <- subset(iris, Species == species), env = list(species = input_value))) + +summary(q[["i"]]$Species) +summary(q[["ii"]]$Species) +summary(q[["iii"]]$Species) +``` + +A more convenient way to pass code with substitution is to use the `within` method. +```{r} +qq <- new_qenv() +qq <- within(qq, i <- subset(iris, Species == "setosa")) +qq <- within(qq, ii <- subset(iris, Species == species), species = "versicolor") +input_value <- "virginica" +qq <- within(qq, iii <- subset(iris, Species == species), species = input_value) + +summary(qq[["i"]]$Species) +summary(qq[["ii"]]$Species) +summary(qq[["iii"]]$Species) +``` + + + To extract objects from a `qenv`, use `[[`; this is particularly useful for displaying them in a `shiny` app. You can retrieve the code used to generate the `qenv` using the `get_code()` function. ```{r} From 1c814086577f7f1a7c3deb3d3091f457d60b78a0 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 4 Oct 2023 16:07:20 +0200 Subject: [PATCH 13/22] improve vignette --- vignettes/qenv.Rmd | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/vignettes/qenv.Rmd b/vignettes/qenv.Rmd index 2af944447..f226ea44e 100644 --- a/vignettes/qenv.Rmd +++ b/vignettes/qenv.Rmd @@ -46,15 +46,29 @@ print(my_qenv) print(q2) ``` +To extract objects from a `qenv`, use `[[`; this is particularly useful for displaying them in a `shiny` app. You can retrieve the code used to generate the `qenv` using the `get_code()` function. + +```{r} +print(q2[["y"]]) + +cat(paste(get_code(q2), collapse = "\n")) +``` +### Substitutions In some cases one may want to substitute some elements of the code before evaluation. Consider a case when a subset of `iris` defined by an input value. ```{r} q <- new_qenv() q <- eval_code(q, quote(i <- subset(iris, Species == "setosa"))) -q <- eval_code(q, substitute(ii <- subset(iris, Species == species), env = list(species = "versicolor"))) +q <- eval_code(q, substitute( + ii <- subset(iris, Species == species), + env = list(species = "versicolor") +)) input_value <- "virginica" -q <- eval_code(q, substitute(iii <- subset(iris, Species == species), env = list(species = input_value))) +q <- eval_code(q, substitute( + iii <- subset(iris, Species == species), + env = list(species = input_value) +)) summary(q[["i"]]$Species) summary(q[["ii"]]$Species) @@ -76,13 +90,6 @@ summary(qq[["iii"]]$Species) -To extract objects from a `qenv`, use `[[`; this is particularly useful for displaying them in a `shiny` app. You can retrieve the code used to generate the `qenv` using the `get_code()` function. - -```{r} -print(q2[["y"]]) - -cat(paste(get_code(q2), collapse = "\n")) -``` ### Combining `qenv` objects Given a pair of `qenv` objects, you may be able to "join" them, creating a new `qenv` object encompassing the union of both environments, along with the requisite code for reproduction: From 2de414983680afecfb18307bd493744a4e1331b5 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 4 Oct 2023 14:10:32 +0000 Subject: [PATCH 14/22] [skip actions] Restyle files --- vignettes/qenv.Rmd | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vignettes/qenv.Rmd b/vignettes/qenv.Rmd index f226ea44e..57e7eb99d 100644 --- a/vignettes/qenv.Rmd +++ b/vignettes/qenv.Rmd @@ -61,12 +61,12 @@ Consider a case when a subset of `iris` defined by an input value. q <- new_qenv() q <- eval_code(q, quote(i <- subset(iris, Species == "setosa"))) q <- eval_code(q, substitute( - ii <- subset(iris, Species == species), + ii <- subset(iris, Species == species), env = list(species = "versicolor") )) input_value <- "virginica" q <- eval_code(q, substitute( - iii <- subset(iris, Species == species), + iii <- subset(iris, Species == species), env = list(species = input_value) )) From cdaa0d64e17b0b53e645b25fd9e4951707549584 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 5 Oct 2023 08:21:42 +0200 Subject: [PATCH 15/22] enhance documentation --- R/qenv-within.R | 12 ++++++++++++ man/within.Rd | 15 +++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/R/qenv-within.R b/R/qenv-within.R index 6d14a15a6..92a4997ec 100644 --- a/R/qenv-within.R +++ b/R/qenv-within.R @@ -7,6 +7,9 @@ #' through the `...` argument: as `name:value` pairs are passed to `...`, #' `name` in `expr` will be replaced with `value`. #' +#' @section Using language objects: +#' Passing language objects to `expr` is generally not intended but can be achieved with `do.call`. +#' Only single `expression`s will work and substitution is not available. See examples. #' #' @param data `qenv` object #' @param expr `expression` to evaluate @@ -45,6 +48,15 @@ #' species_external <- "versicolor" #' within(q, print(dim(subset(i, Species == species))), species = species_external) #' +#' # pass language objects +#' expr <- expression(i <- iris, m <- mtcars) +#' within(q, expr) # fails +#' do.call(within, list(q, expr)) +#' +#' exprlist <- list(expression(i <- iris), expression(m <- mtcars)) +#' within(q, exprlist) # fails +#' do.call(within, list(q, do.call(c, exprlist))) +#' within.qenv <- function(data, expr, ...) { expr <- substitute(expr) extras <- list(...) diff --git a/man/within.Rd b/man/within.Rd index 5cab9155b..bb8cc449f 100644 --- a/man/within.Rd +++ b/man/within.Rd @@ -25,6 +25,12 @@ It accepts only inline expressions (both simple and compound) and allows for inj through the \code{...} argument: as \code{name:value} pairs are passed to \code{...}, \code{name} in \code{expr} will be replaced with \code{value}. } +\section{Using language objects}{ + +Passing language objects to \code{expr} is generally not intended but can be achieved with \code{do.call}. +Only single \code{expression}s will work and substitution is not available. See examples. +} + \examples{ q <- new_qenv() @@ -49,6 +55,15 @@ within(q, print(dim(subset(i, Species == species))), species = "versicolor") species_external <- "versicolor" within(q, print(dim(subset(i, Species == species))), species = species_external) +# pass language objects +expr <- expression(i <- iris, m <- mtcars) +within(q, expr) # fails +do.call(within, list(q, expr)) + +exprlist <- list(expression(i <- iris), expression(m <- mtcars)) +within(q, exprlist) # fails +do.call(within, list(q, do.call(c, exprlist))) + } \seealso{ \code{\link{eval_code}}, \code{\link[base:with]{base::within}} From 72b762bd08e270f2c83b455888a85b65ec5cfec3 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 5 Oct 2023 10:32:34 +0200 Subject: [PATCH 16/22] remove warning for symbols --- R/qenv-within.R | 11 ----------- tests/testthat/test-qenv-within.R | 10 ---------- 2 files changed, 21 deletions(-) diff --git a/R/qenv-within.R b/R/qenv-within.R index 92a4997ec..fd0b2de61 100644 --- a/R/qenv-within.R +++ b/R/qenv-within.R @@ -61,17 +61,6 @@ within.qenv <- function(data, expr, ...) { expr <- substitute(expr) extras <- list(...) - # Get parent call to use in error messages. - parent_call <- deparse1(match.call(definition = sys.function(1L), call = sys.call(1L))) - - if (is.symbol(expr)) { - warning( - "In ", parent_call, " : Symbol passed to \"expr\" will be evaluated within qenv as is. ", "\n", - "This function is intended for literal expressions, for variables containing code use `eval_code` instead.", - call. = FALSE - ) - } - # Add braces for consistency. if (!identical(as.list(expr)[[1L]], as.symbol("{"))) { expr <- call("{", expr) diff --git a/tests/testthat/test-qenv-within.R b/tests/testthat/test-qenv-within.R index c21cfb3c6..40c8ecca5 100644 --- a/tests/testthat/test-qenv-within.R +++ b/tests/testthat/test-qenv-within.R @@ -50,16 +50,6 @@ testthat::test_that("character-only compound expressions are ignored", { testthat::expect_equal(q, qq) }) -testthat::test_that("symbol passed to `expr` raises warning", { - q <- new_qenv() - q <- within(q, i <- iris) - testthat::expect_warning( - qq <- within(q, i), - "Symbol passed to \"expr\"" - ) - testthat::expect_identical(get_code(qq), c("i <- iris", "i")) -}) - ## code identity ---- testthat::test_that("differently formulated expressions yield the same code", { From 00ae3dad3d66f200a4b2733a2e8bbf75fd8d48d4 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 5 Oct 2023 10:36:12 +0200 Subject: [PATCH 17/22] remove dropping strings from expr --- R/qenv-within.R | 4 ++-- tests/testthat/test-qenv-within.R | 16 ---------------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/R/qenv-within.R b/R/qenv-within.R index fd0b2de61..a6413568e 100644 --- a/R/qenv-within.R +++ b/R/qenv-within.R @@ -65,8 +65,8 @@ within.qenv <- function(data, expr, ...) { if (!identical(as.list(expr)[[1L]], as.symbol("{"))) { expr <- call("{", expr) } - # Drop strings from compound expressions. - calls <- Filter(Negate(is.character), as.list(expr)[-1]) + + calls <- as.list(expr)[-1] # Inject extra values into expressions. calls <- lapply(calls, function(x) do.call(substitute, list(x, env = extras))) diff --git a/tests/testthat/test-qenv-within.R b/tests/testthat/test-qenv-within.R index 40c8ecca5..74eff700c 100644 --- a/tests/testthat/test-qenv-within.R +++ b/tests/testthat/test-qenv-within.R @@ -34,22 +34,6 @@ testthat::test_that("compound expressions are evaluated", { ) }) -testthat::test_that("characters in compound expressions are omitted", { - q <- new_qenv() - qq <- within(q, { - i <- iris - "1 + 1" - m <- mtcars - }) - testthat::expect_identical(get_code(qq), c("i <- iris", "m <- mtcars")) -}) - -testthat::test_that("character-only compound expressions are ignored", { - q <- new_qenv() - qq <- within(q, {"1 + 1"}) - testthat::expect_equal(q, qq) -}) - ## code identity ---- testthat::test_that("differently formulated expressions yield the same code", { From 668bcda1186b731c7396759d783092b89799cf47 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 5 Oct 2023 10:42:30 +0200 Subject: [PATCH 18/22] improve tests for value injection --- tests/testthat/test-qenv-within.R | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/tests/testthat/test-qenv-within.R b/tests/testthat/test-qenv-within.R index 74eff700c..077226357 100644 --- a/tests/testthat/test-qenv-within.R +++ b/tests/testthat/test-qenv-within.R @@ -79,45 +79,42 @@ testthat::test_that("differently formulated expressions yield the same code", { testthat::test_that("external values can be injected into expressions through `...`", { q <- new_qenv() - q <- within(q, { - i <- subset(iris, Species == "setosa") + q1 <- within(q, { + i <- subset(iris, Species == "virginica") }) - q <- within(q, { - ii <- subset(iris, Species == species) + q2 <- within(q, { + i <- subset(iris, Species == species) }, species = "virginica") - external_value <- "versicolor" - q <- within(q, { - iii <- subset(iris, Species == species) + external_value <- "virginica" + q3 <- within(q, { + i <- subset(iris, Species == species) }, species = external_value) - testthat::expect_identical( - get_code(q), - c( - "i <- subset(iris, Species == \"setosa\")", - "ii <- subset(iris, Species == \"virginica\")", - "iii <- subset(iris, Species == \"versicolor\")" - ) - ) + testthat::expect_identical(get_code(q1), get_code(q2)) + testthat::expect_identical(get_code(q1), get_code(q3)) +}) +testthat::test_that("external values are not taken from calling frame", { q <- new_qenv() species <- "setosa" qq <- within(q, { i <- subset(iris, Species == species) }) testthat::expect_s3_class(qq, "qenv.error") + testthat::expect_error(get_code(qq), "object 'species' not found") qq <- within(q, { i <- subset(iris, Species == species) }, species = species) testthat::expect_s4_class(qq, "qenv") + testthat::expect_identical(get_code(qq), "i <- subset(iris, Species == \"setosa\")") }) - # return value ---- testthat::test_that("within.qenv renturns a deep copy of `data`", { q <- new_qenv() From e87a418c524b01a6c214a6345342a45316315aad Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 5 Oct 2023 10:58:12 +0200 Subject: [PATCH 19/22] improve test description --- tests/testthat/test-qenv-within.R | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/tests/testthat/test-qenv-within.R b/tests/testthat/test-qenv-within.R index 077226357..2f8261d8e 100644 --- a/tests/testthat/test-qenv-within.R +++ b/tests/testthat/test-qenv-within.R @@ -3,40 +3,20 @@ # evaluation ---- ## code acceptance ---- -testthat::test_that("simple expressions are evaluated", { - q <- new_qenv() - testthat::expect_no_error(within(q, 1 + 1)) -}) - -testthat::test_that("compound expressions are evaluated", { +testthat::test_that("simple and compound expressions are evaluated", { q <- new_qenv() testthat::expect_no_error( - within(q, { - 1 + 1 - }) + within(q, 1 + 1) ) testthat::expect_no_error( within(q, { 1 + 1 - 2 + 2 - }) - ) - testthat::expect_no_error( - within(q, { - 1 + 1; 2 + 2 - }) - ) - testthat::expect_no_error( - within(q, { - 1 + - 1 }) ) }) - ## code identity ---- -testthat::test_that("differently formulated expressions yield the same code", { +testthat::test_that("styling of input code does not impact evaluation results", { q <- new_qenv() q <- within(q, 1 + 1) q <- within(q, {1 + 1}) @@ -67,7 +47,6 @@ testthat::test_that("differently formulated expressions yield the same code", { 2 + 2 }) all_code <- get_code(q) - all_code_pairs <- lapply(seq_len(4L), function(x) all_code[((x - 1L) * 2L) + 1:2]) testthat::expect_identical( all_code, rep(c("1 + 1", "2 + 2"), 4L) From 666d095c842e550346e730b3182b6c7302cc69ea Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 5 Oct 2023 11:02:28 +0200 Subject: [PATCH 20/22] add note in vignette --- vignettes/qenv.Rmd | 1 + 1 file changed, 1 insertion(+) diff --git a/vignettes/qenv.Rmd b/vignettes/qenv.Rmd index 57e7eb99d..5b0a8c73e 100644 --- a/vignettes/qenv.Rmd +++ b/vignettes/qenv.Rmd @@ -88,6 +88,7 @@ summary(qq[["ii"]]$Species) summary(qq[["iii"]]$Species) ``` +See `?within.qenv` for more details. ### Combining `qenv` objects From ee0558b38ee1bc9beee2af171b5dc20186a85371 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 5 Oct 2023 11:11:39 +0200 Subject: [PATCH 21/22] improve tests for return value --- tests/testthat/test-qenv-within.R | 52 ++++++++++++++----------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/tests/testthat/test-qenv-within.R b/tests/testthat/test-qenv-within.R index 2f8261d8e..46ec6181f 100644 --- a/tests/testthat/test-qenv-within.R +++ b/tests/testthat/test-qenv-within.R @@ -1,8 +1,7 @@ # styler: off # nolint start -# evaluation ---- -## code acceptance ---- +# code acceptance ---- testthat::test_that("simple and compound expressions are evaluated", { q <- new_qenv() testthat::expect_no_error( @@ -15,7 +14,7 @@ testthat::test_that("simple and compound expressions are evaluated", { ) }) -## code identity ---- +# code identity ---- testthat::test_that("styling of input code does not impact evaluation results", { q <- new_qenv() q <- within(q, 1 + 1) @@ -54,7 +53,27 @@ testthat::test_that("styling of input code does not impact evaluation results", }) -## injecting values ---- +# return value ---- +testthat::test_that("within.qenv renturns a deep copy of `data`", { + q <- new_qenv() + q <- within(new_qenv(), i <- iris) + qq <- within(q, {}) + testthat::expect_equal(q, qq) + testthat::expect_false(identical(q, qq)) +}) + +testthat::test_that("within.qenv renturns qenv.error even if evaluation raises error", { + q <- new_qenv() + q <- within(q, i <- iris) + qq <- within(q, stop("right there")) + testthat::expect_true( + exists("qq", inherits = FALSE) + ) + testthat::expect_s3_class(qq, "qenv.error") +}) + + +# injecting values ---- testthat::test_that("external values can be injected into expressions through `...`", { q <- new_qenv() @@ -94,30 +113,5 @@ testthat::test_that("external values are not taken from calling frame", { testthat::expect_identical(get_code(qq), "i <- subset(iris, Species == \"setosa\")") }) -# return value ---- -testthat::test_that("within.qenv renturns a deep copy of `data`", { - q <- new_qenv() - q <- within(new_qenv(), i <- iris) - qq <- within(q, {}) - testthat::expect_equal(q, qq) - - q <- new_qenv() - q <- within(q, i <- iris) - qq <- within(q, m <- mtcars) - testthat::expect_failure( - testthat::expect_equal(q, qq) - ) -}) - -testthat::test_that("within.qenv renturns qenv.error even if evaluation raises error", { - q <- new_qenv() - q <- within(q, i <- iris) - qq <- within(q, stop("right there")) - testthat::expect_true( - exists("qq", inherits = FALSE) - ) - testthat::expect_s3_class(qq, "qenv.error") -}) - # nolint end # styler: on From eea05131567d526589d76f9966775d954cc6df2d Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 5 Oct 2023 12:08:28 +0200 Subject: [PATCH 22/22] reduce tests --- tests/testthat/test-qenv-within.R | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/testthat/test-qenv-within.R b/tests/testthat/test-qenv-within.R index 46ec6181f..05ab6c1fa 100644 --- a/tests/testthat/test-qenv-within.R +++ b/tests/testthat/test-qenv-within.R @@ -77,23 +77,13 @@ testthat::test_that("within.qenv renturns qenv.error even if evaluation raises e testthat::test_that("external values can be injected into expressions through `...`", { q <- new_qenv() - q1 <- within(q, { - i <- subset(iris, Species == "virginica") - }) - - q2 <- within(q, { - i <- subset(iris, Species == species) - }, - species = "virginica") - external_value <- "virginica" - q3 <- within(q, { + q <- within(q, { i <- subset(iris, Species == species) }, species = external_value) - testthat::expect_identical(get_code(q1), get_code(q2)) - testthat::expect_identical(get_code(q1), get_code(q3)) + testthat::expect_identical(get_code(q), "i <- subset(iris, Species == \"virginica\")") }) testthat::test_that("external values are not taken from calling frame", {