From 51e46bd2007ea7955849af02b3f41f8b64d41f93 Mon Sep 17 00:00:00 2001 From: m7pr Date: Fri, 31 Oct 2025 09:59:33 +0100 Subject: [PATCH 1/9] handle subassignemnts with the same object --- R/utils-get_code_dependency.R | 16 +- .../testthat/test-utils-get_code_dependency.R | 171 ++++++++++++++++++ 2 files changed, 186 insertions(+), 1 deletion(-) diff --git a/R/utils-get_code_dependency.R b/R/utils-get_code_dependency.R index 42e0650ad..0690abe92 100644 --- a/R/utils-get_code_dependency.R +++ b/R/utils-get_code_dependency.R @@ -306,7 +306,18 @@ extract_occurrence <- function(pd) { ans <- move_functions_after_arrow(ans, unique(x[sym_fc_cond, "text"])) roll <- in_parenthesis(pd) if (length(roll)) { - c(setdiff(ans, roll), roll) + # detect elements appread in parenthesis and move them on RHS + # but only their first appearance + # as the same object can appear as regular object and the one used in parenthesis + result <- ans + for (elem in roll) { + idx <- which(result == elem)[1] + if (!is.na(idx)) { + result <- result[-idx] + } + } + c(result, roll) + } else { ans } @@ -330,6 +341,9 @@ move_functions_after_arrow <- function(ans, functions) { if (length(arrow_pos) == 0) { return(ans) } + if (length(functions) == 0) { + return(ans) + } before_arrow <- setdiff(ans[1:arrow_pos], functions) after_arrow <- ans[(arrow_pos + 1):length(ans)] c(before_arrow, unique(c(intersect(ans[1:arrow_pos], functions), after_arrow))) diff --git a/tests/testthat/test-utils-get_code_dependency.R b/tests/testthat/test-utils-get_code_dependency.R index 2cd4c40cd..c5b0b272e 100644 --- a/tests/testthat/test-utils-get_code_dependency.R +++ b/tests/testthat/test-utils-get_code_dependency.R @@ -86,3 +86,174 @@ testthat::describe("get_code with multiple assignments inside an expression", { testthat::expect_equal(get_code(td, names = "var2"), code_source) }) }) + +testthat::describe("get_code with subassignments", { + testthat::it("tracks $ subassignment as producing the base object", { + td <- qenv() |> + within({ + iris <- iris + iris$Species[sample.int(nrow(iris), 50)] <- NA + }) + + code_source <- "iris <- iris\niris$Species[sample.int(nrow(iris), 50)] <- NA" + + testthat::expect_equal(get_code(td, names = "iris"), code_source) + }) + + testthat::it("tracks [ subassignment as producing the base object", { + td <- qenv() |> + within({ + x <- 1:10 + x[1:3] <- c(10, 20, 30) + }) + + code_source <- "x <- 1:10\nx[1:3] <- c(10, 20, 30)" + + testthat::expect_equal(get_code(td, names = "x"), code_source) + }) + + testthat::it("tracks [[ subassignment as producing the base object", { + td <- qenv() |> + within({ + lst <- list(a = 1, b = 2) + lst[["c"]] <- 3 + }) + + code_source <- "lst <- list(a = 1, b = 2)\nlst[[\"c\"]] <- 3" + + testthat::expect_equal(get_code(td, names = "lst"), code_source) + }) + + testthat::it("tracks nested subassignments", { + td <- qenv() |> + within({ + df <- data.frame(x = 1:5, y = 6:10) + df$x[df$y > 8] <- 99 + }) + + code_source <- "df <- data.frame(x = 1:5, y = 6:10)\ndf$x[df$y > 8] <- 99" + + testthat::expect_equal(get_code(td, names = "df"), code_source) + }) + + testthat::it("tracks multiple subassignments to same object", { + td <- qenv() |> + within({ + iris <- iris + iris$Species[sample.int(nrow(iris), 10)] <- NA + iris$Sepal.Length[1:5] <- 0 + }) + + code_source <- "iris <- iris\niris$Species[sample.int(nrow(iris), 10)] <- NA\niris$Sepal.Length[1:5] <- 0" + + testthat::expect_equal(get_code(td, names = "iris"), code_source) + }) + + testthat::it("tracks subassignments with complex expressions", { + td <- qenv() |> + within({ + mat <- matrix(1:12, nrow = 3) + mat[mat > 5 & mat < 10] <- 0 + }) + + code_source <- "mat <- matrix(1:12, nrow = 3)\nmat[mat > 5 & mat < 10] <- 0" + + testthat::expect_equal(get_code(td, names = "mat"), code_source) + }) + + testthat::it("tracks subassignments with function calls on LHS", { + td <- qenv() |> + within({ + lst <- list(a = 1, b = 2) + names(lst)[1] <- "first" + }) + + code_source <- "lst <- list(a = 1, b = 2)\nnames(lst)[1] <- \"first\"" + + testthat::expect_equal(get_code(td, names = "lst"), code_source) + }) + + testthat::it("tracks -> operator with subassignments", { + td <- qenv() |> + within({ + x <- 1:10 + c(10, 20, 30) -> x[1:3] # nolint: assignment. + }) + + code_source <- "x <- 1:10\nx[1:3] <- c(10, 20, 30)" + + testthat::expect_equal(get_code(td, names = "x"), code_source) + }) + + testthat::it("tracks attributes() function with subassignments", { + td <- qenv() |> + within({ + x <- 1:5 + attributes(x)$names <- letters[1:5] + }) + + code_source <- "x <- 1:5\nattributes(x)$names <- letters[1:5]" + + testthat::expect_equal(get_code(td, names = "x"), code_source) + }) + + testthat::it("handles complex nested subassignments", { + td <- qenv() |> + within({ + df <- data.frame(x = 1:5, y = 6:10) + df[df$x > 2, "y"][1:2] <- c(99, 100) + }) + + code_source <- "df <- data.frame(x = 1:5, y = 6:10)\ndf[df$x > 2, \"y\"][1:2] <- c(99, 100)" + + testthat::expect_equal(get_code(td, names = "df"), code_source) + }) + + testthat::it("handles subassignments with multiple operators", { + td <- qenv() |> + within({ + lst <- list(a = list(b = 1, c = 2)) + lst$a$b[2] <- 99 + }) + + code_source <- "lst <- list(a = list(b = 1, c = 2))\nlst$a$b[2] <- 99" + + testthat::expect_equal(get_code(td, names = "lst"), code_source) + }) + + testthat::it("handles subassignments with data frame column creation", { + td <- qenv() |> + within({ + df <- data.frame(x = 1:3) + df$new_col <- c("a", "b", "c") + }) + + code_source <- "df <- data.frame(x = 1:3)\ndf$new_col <- c(\"a\", \"b\", \"c\")" + + testthat::expect_equal(get_code(td, names = "df"), code_source) + }) + + testthat::it("handles subassignments with matrix indexing", { + td <- qenv() |> + within({ + mat <- matrix(1:9, nrow = 3) + mat[1:2, 2:3] <- matrix(0, nrow = 2, ncol = 2) + }) + + code_source <- "mat <- matrix(1:9, nrow = 3)\nmat[1:2, 2:3] <- matrix(0, nrow = 2, ncol = 2)" + + testthat::expect_equal(get_code(td, names = "mat"), code_source) + }) + + testthat::it("handles subassignments with logical indexing", { + td <- qenv() |> + within({ + vec <- 1:10 + vec[vec %% 2 == 0] <- vec[vec %% 2 == 0] * 2 + }) + + code_source <- "vec <- 1:10\nvec[vec %% 2 == 0] <- vec[vec %% 2 == 0] * 2" + + testthat::expect_equal(get_code(td, names = "vec"), code_source) + }) +}) From 9b15b77d759a66409cdfa0bdc36addc06ca21a13 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 31 Oct 2025 09:02:41 +0000 Subject: [PATCH 2/9] [skip style] [skip vbump] Restyle files --- R/utils-get_code_dependency.R | 1 - 1 file changed, 1 deletion(-) diff --git a/R/utils-get_code_dependency.R b/R/utils-get_code_dependency.R index 0690abe92..d1cbcbdaf 100644 --- a/R/utils-get_code_dependency.R +++ b/R/utils-get_code_dependency.R @@ -317,7 +317,6 @@ extract_occurrence <- function(pd) { } } c(result, roll) - } else { ans } From af2fbbb3c57ad929efc165b3d946c663fdeed64c Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 31 Oct 2025 09:05:31 +0000 Subject: [PATCH 3/9] [skip roxygen] [skip vbump] Roxygen Man Pages Auto Update --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 74200b457..4b068d46f 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -53,7 +53,7 @@ Config/Needs/website: insightsengineering/nesttemplate Encoding: UTF-8 Language: en-US Roxygen: list(markdown = TRUE) -RoxygenNote: 7.3.2 +RoxygenNote: 7.3.3 Collate: 'qenv-c.R' 'qenv-class.R' From 24a3f6e78078360bfd689101013ac42cd5226496 Mon Sep 17 00:00:00 2001 From: m7pr Date: Fri, 31 Oct 2025 10:19:07 +0100 Subject: [PATCH 4/9] cleanup tests --- tests/testthat/test-utils-get_code_dependency.R | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/tests/testthat/test-utils-get_code_dependency.R b/tests/testthat/test-utils-get_code_dependency.R index c5b0b272e..db06dcdc1 100644 --- a/tests/testthat/test-utils-get_code_dependency.R +++ b/tests/testthat/test-utils-get_code_dependency.R @@ -88,18 +88,6 @@ testthat::describe("get_code with multiple assignments inside an expression", { }) testthat::describe("get_code with subassignments", { - testthat::it("tracks $ subassignment as producing the base object", { - td <- qenv() |> - within({ - iris <- iris - iris$Species[sample.int(nrow(iris), 50)] <- NA - }) - - code_source <- "iris <- iris\niris$Species[sample.int(nrow(iris), 50)] <- NA" - - testthat::expect_equal(get_code(td, names = "iris"), code_source) - }) - testthat::it("tracks [ subassignment as producing the base object", { td <- qenv() |> within({ @@ -252,7 +240,7 @@ testthat::describe("get_code with subassignments", { vec[vec %% 2 == 0] <- vec[vec %% 2 == 0] * 2 }) - code_source <- "vec <- 1:10\nvec[vec %% 2 == 0] <- vec[vec %% 2 == 0] * 2" + code_source <- "vec <- 1:10\nvec[vec%%2 == 0] <- vec[vec%%2 == 0] * 2" testthat::expect_equal(get_code(td, names = "vec"), code_source) }) From 0ba04ccc8f70df177f098ba727d9be6f56839944 Mon Sep 17 00:00:00 2001 From: m7pr Date: Fri, 31 Oct 2025 10:48:33 +0100 Subject: [PATCH 5/9] fix move_functions_after_arrow --- R/utils-get_code_dependency.R | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/R/utils-get_code_dependency.R b/R/utils-get_code_dependency.R index d1cbcbdaf..7836ff01b 100644 --- a/R/utils-get_code_dependency.R +++ b/R/utils-get_code_dependency.R @@ -343,9 +343,14 @@ move_functions_after_arrow <- function(ans, functions) { if (length(functions) == 0) { return(ans) } - before_arrow <- setdiff(ans[1:arrow_pos], functions) - after_arrow <- ans[(arrow_pos + 1):length(ans)] - c(before_arrow, unique(c(intersect(ans[1:arrow_pos], functions), after_arrow))) + ans_pre <- ans[1:arrow_pos] + for (fun in functions) { + if (any(ans_pre == fun)) ans_pre <- ans_pre[-match(fun, ans_pre)] + } + after_arrow <- if (arrow_pos < length(ans)) { + ans[arrow_pos + 1:length(ans)] + } + c(ans_pre, after_arrow) } #' Extract side effects From 98b7597b509293c0582caaaef271ecf239f54d89 Mon Sep 17 00:00:00 2001 From: m7pr Date: Fri, 31 Oct 2025 10:57:11 +0100 Subject: [PATCH 6/9] add comments --- R/utils-get_code_dependency.R | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/R/utils-get_code_dependency.R b/R/utils-get_code_dependency.R index 7836ff01b..a1ea0b86e 100644 --- a/R/utils-get_code_dependency.R +++ b/R/utils-get_code_dependency.R @@ -344,6 +344,10 @@ move_functions_after_arrow <- function(ans, functions) { return(ans) } ans_pre <- ans[1:arrow_pos] + # it's setdiff but without the removal of duplicates + # do not use setdiff(ans_pre, functions) + # as it removes duplicates from ans_pre even if they do not appear in functions + # check setdiff(c("A", "A"), "B") - gives "A", where we want to keep c("A", "A") for (fun in functions) { if (any(ans_pre == fun)) ans_pre <- ans_pre[-match(fun, ans_pre)] } From 2a4a6cf9e632ef5cb7255d3bcd2d730a110326a2 Mon Sep 17 00:00:00 2001 From: Marcin <133694481+m7pr@users.noreply.github.com> Date: Fri, 31 Oct 2025 11:22:33 +0100 Subject: [PATCH 7/9] Update R/utils-get_code_dependency.R Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com> --- R/utils-get_code_dependency.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/utils-get_code_dependency.R b/R/utils-get_code_dependency.R index a1ea0b86e..237c168a9 100644 --- a/R/utils-get_code_dependency.R +++ b/R/utils-get_code_dependency.R @@ -352,7 +352,7 @@ move_functions_after_arrow <- function(ans, functions) { if (any(ans_pre == fun)) ans_pre <- ans_pre[-match(fun, ans_pre)] } after_arrow <- if (arrow_pos < length(ans)) { - ans[arrow_pos + 1:length(ans)] + ans[(arrow_pos + 1):length(ans)] } c(ans_pre, after_arrow) } From 7ca59f979263e39487094b389f06c643fc230542 Mon Sep 17 00:00:00 2001 From: Marcin <133694481+m7pr@users.noreply.github.com> Date: Fri, 31 Oct 2025 11:22:53 +0100 Subject: [PATCH 8/9] Update R/utils-get_code_dependency.R Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com> --- R/utils-get_code_dependency.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/utils-get_code_dependency.R b/R/utils-get_code_dependency.R index 237c168a9..954448d31 100644 --- a/R/utils-get_code_dependency.R +++ b/R/utils-get_code_dependency.R @@ -306,7 +306,7 @@ extract_occurrence <- function(pd) { ans <- move_functions_after_arrow(ans, unique(x[sym_fc_cond, "text"])) roll <- in_parenthesis(pd) if (length(roll)) { - # detect elements appread in parenthesis and move them on RHS + # detect elements appeared in parenthesis and move them on RHS # but only their first appearance # as the same object can appear as regular object and the one used in parenthesis result <- ans From 3a6ece30e07c944289e3ab901d9d053956a8ea5d Mon Sep 17 00:00:00 2001 From: Marcin <133694481+m7pr@users.noreply.github.com> Date: Fri, 31 Oct 2025 13:52:01 +0100 Subject: [PATCH 9/9] Apply suggestions from code review Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com> --- .../testthat/test-utils-get_code_dependency.R | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/testthat/test-utils-get_code_dependency.R b/tests/testthat/test-utils-get_code_dependency.R index db06dcdc1..a9f18f0e8 100644 --- a/tests/testthat/test-utils-get_code_dependency.R +++ b/tests/testthat/test-utils-get_code_dependency.R @@ -88,7 +88,7 @@ testthat::describe("get_code with multiple assignments inside an expression", { }) testthat::describe("get_code with subassignments", { - testthat::it("tracks [ subassignment as producing the base object", { + it("tracks [ subassignment as producing the base object", { td <- qenv() |> within({ x <- 1:10 @@ -100,7 +100,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "x"), code_source) }) - testthat::it("tracks [[ subassignment as producing the base object", { + it("tracks [[ subassignment as producing the base object", { td <- qenv() |> within({ lst <- list(a = 1, b = 2) @@ -112,7 +112,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "lst"), code_source) }) - testthat::it("tracks nested subassignments", { + it("tracks nested subassignments", { td <- qenv() |> within({ df <- data.frame(x = 1:5, y = 6:10) @@ -124,7 +124,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "df"), code_source) }) - testthat::it("tracks multiple subassignments to same object", { + it("tracks multiple subassignments to same object", { td <- qenv() |> within({ iris <- iris @@ -137,7 +137,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "iris"), code_source) }) - testthat::it("tracks subassignments with complex expressions", { + it("tracks subassignments with complex expressions", { td <- qenv() |> within({ mat <- matrix(1:12, nrow = 3) @@ -149,7 +149,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "mat"), code_source) }) - testthat::it("tracks subassignments with function calls on LHS", { + it("tracks subassignments with function calls on LHS", { td <- qenv() |> within({ lst <- list(a = 1, b = 2) @@ -161,7 +161,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "lst"), code_source) }) - testthat::it("tracks -> operator with subassignments", { + it("tracks -> operator with subassignments", { td <- qenv() |> within({ x <- 1:10 @@ -173,7 +173,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "x"), code_source) }) - testthat::it("tracks attributes() function with subassignments", { + it("tracks attributes() function with subassignments", { td <- qenv() |> within({ x <- 1:5 @@ -185,7 +185,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "x"), code_source) }) - testthat::it("handles complex nested subassignments", { + it("handles complex nested subassignments", { td <- qenv() |> within({ df <- data.frame(x = 1:5, y = 6:10) @@ -197,7 +197,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "df"), code_source) }) - testthat::it("handles subassignments with multiple operators", { + it("handles subassignments with multiple operators", { td <- qenv() |> within({ lst <- list(a = list(b = 1, c = 2)) @@ -209,7 +209,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "lst"), code_source) }) - testthat::it("handles subassignments with data frame column creation", { + it("handles subassignments with data frame column creation", { td <- qenv() |> within({ df <- data.frame(x = 1:3) @@ -221,7 +221,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "df"), code_source) }) - testthat::it("handles subassignments with matrix indexing", { + it("handles subassignments with matrix indexing", { td <- qenv() |> within({ mat <- matrix(1:9, nrow = 3) @@ -233,7 +233,7 @@ testthat::describe("get_code with subassignments", { testthat::expect_equal(get_code(td, names = "mat"), code_source) }) - testthat::it("handles subassignments with logical indexing", { + it("handles subassignments with logical indexing", { td <- qenv() |> within({ vec <- 1:10