fixes

lvaudor · Oct 12, 2023 · d8e3620 · d8e3620
1 parent 4007511
commit d8e3620
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 7 deletions.
diff --git a/R/spq_mutate.R b/R/spq_mutate.R
@@ -53,8 +53,6 @@ spq_mutate = function(.query, ..., .label = NA, .within_box = c(NA, NA), .within
   for (var in normal_variables) {
     name = sprintf("?%s", names(normal_variables)[normal_variables == var])
 
-    .query = spq_select(.query, spq(name))
-
     formula_df = get_varformula(var)
     .query = track_vars(
       .query = .query,
@@ -64,6 +62,8 @@ spq_mutate = function(.query, ..., .label = NA, .within_box = c(NA, NA), .within
       fun = sub("\\)$", "", sub("\\(.*", "", formula_df[["formula"]]))
     )
 
+    .query = track_structure(.query, name = name, selected = TRUE)
+
   }
 
   # 'Triple' variables

diff --git a/R/spq_select.R b/R/spq_select.R
@@ -71,7 +71,7 @@ check_variables_present <- function(query, variables) {
     return()
   }
 
-  absent_variables <- setdiff(variables, query[["structure"]][["name"]])
+  absent_variables <- setdiff(variables, query[["vars"]][["name"]])
 
   if (length(absent_variables) > 0) {
     cli::cli_abort(c(

diff --git a/R/spq_summarise.R b/R/spq_summarise.R
@@ -81,7 +81,7 @@ spq_summarise = function(.query, ...) {
       ancestor = ancestor,
       fun = fun
     )
-    .query = spq_select(.query, spq(name))
+    .query = track_structure(.query, name, selected = TRUE)
 
   }
 

diff --git a/R/spq_tally.R b/R/spq_tally.R
@@ -57,13 +57,13 @@ spq_tally = function(.query, sort = FALSE, name = "n") {
 
   .query = track_vars(
     .query,
-    name = sprintf("?%s", name),
+    name = question_mark(name),
     formula = full_formula,
     fun = "COUNT",
     ancestor = "*"
   )
 
-  .query = spq_select(.query, name)
+  .query = track_structure(.query, name = question_mark(name), selected = TRUE)
 
   if (sort) {
     .query <- spq_arrange(.query, spq(sprintf("DESC(?%s)", name)))

diff --git a/tests/testthat/_snaps/spq_select.md b/tests/testthat/_snaps/spq_select.md
@@ -154,12 +154,23 @@
 
 # spq_select tells a variable isn't there
 
+    Code
+      spq_init() %>% spq_add("?station wdt:P16 wd:Q1552") %>% spq_add(
+        "?station wdt:P31 wd:Q928830") %>% spq_add("?station wdt:P625 ?coords") %>%
+        spq_select(station_label, blop)
+    Condition
+      Error in `check_variables_present()`:
+      ! Can't use `spq_select()` on absent variables: ?station_label, ?blop.
+      i Did you forget a call to `spq_add()`, `spq_mutate()` or `spq_label()`?
+
+---
+
     Code
       spq_init() %>% spq_add("?station wdt:P16 wd:Q1552") %>% spq_add(
         "?station wdt:P31 wd:Q928830") %>% spq_add("?station wdt:P625 ?coords") %>%
         spq_label(station) %>% spq_select(station_label, blop)
     Condition
       Error in `check_variables_present()`:
-      ! Can't use `spq_select()` on absent variables: ?station_label.
+      ! Can't use `spq_select()` on absent variables: ?blop.
       i Did you forget a call to `spq_add()`, `spq_mutate()` or `spq_label()`?
 
diff --git a/tests/testthat/test-spq_group_by.R b/tests/testthat/test-spq_group_by.R
@@ -1,4 +1,6 @@
 test_that("spq_group_by works with R syntax", {
+  withr::local_envvar("GLITTER.TESTING.SELECT" = "yep")
+
   expect_snapshot(
     spq_init() %>%
       spq_select(population, countryLabel) %>%
@@ -7,6 +9,8 @@ test_that("spq_group_by works with R syntax", {
 })
 
 test_that("spq_group_by works with R syntax - string", {
+  withr::local_envvar("GLITTER.TESTING.SELECT" = "yep")
+
   expect_snapshot(
     spq_init() %>%
       spq_select(population, countryLabel) %>%

diff --git a/tests/testthat/test-spq_select.R b/tests/testthat/test-spq_select.R
@@ -76,6 +76,14 @@ test_that("spq_select can use DISTINCT and REDUCED", {
 })
 
 test_that("spq_select tells a variable isn't there", {
+  expect_snapshot(error = TRUE, {
+    spq_init() %>%
+      spq_add("?station wdt:P16 wd:Q1552") %>%
+      spq_add("?station wdt:P31 wd:Q928830") %>%
+      spq_add("?station wdt:P625 ?coords") %>%
+      spq_select(station_label, blop)
+  })
+
   expect_snapshot(error = TRUE, {
     spq_init() %>%
       spq_add("?station wdt:P16 wd:Q1552") %>%