combine_supp() requires that the QNAM columns are not in the source…

… dataset

NEWS.md

@@ -1,4 +1,5 @@
 # metatools 0.1.5.9000
+* Breaking change: `combine_supp()` requires that the QNAM columns are not in the source dataset (#64)
 * Allow supp data to be zero-row with `combine_supp()` (fix #45)
 
 # metatools 0.1.4

R/supp.R

@@ -156,8 +156,15 @@ combine_supp <- function(dataset, supp){
                      "")
       stop(paste0(mess, ext, mis))
    }
-   by <- names(dataset) %>%
-      discard(~ . %in% supp$QNAM) # Don't want any variables in our by statement
+   all_qnam <- unique(supp$QNAM)
+   existing_qnam <- intersect(all_qnam, names(dataset))
+   if (length(existing_qnam) > 0) {
+     stop(
+       "The following column(s) would be created by combine_supp(), but are already in the original dataset:\n  ",
+       paste(existing_qnam, sep = ", ")
+     )
+   }
+   by <- names(dataset)
 
    # In order to prevent issues when there are multiple IDVARS we need to merge
    # each IDVAR into the domain seperately (otherwise there is problems when the

tests/testthat/test-supp.R

@@ -173,14 +173,11 @@ test_that("combine_supp", {
    dataset = ae %>%
       select(-starts_with("SUPP"))
    supp = suppae
-   multi_out <- combine_supp(ae, suppae) %>%
-      dplyr::summarise(v1 = all(all.equal(SUPPVAR1.x, SUPPVAR1.y)), #Because there are NA rows
-                v2 = all(all.equal(SUPPVAR2.x, SUPPVAR2.y)),
-                v3 = all(SUPPVAR3.x == SUPPVAR3.y)) %>%
-      tidyr::pivot_longer(everything()) %>%
-      pull(value) %>%
-      all()
-   expect_equal(multi_out, TRUE)
+
+   multi_out <- combine_supp(dataset, suppae)
+   expect_equal(multi_out$SUPPVAR1, ae$SUPPVAR1)
+   expect_equal(multi_out$SUPPVAR2, ae$SUPPVAR2)
+   expect_equal(multi_out$SUPPVAR3, ae$SUPPVAR3)
 })
 
 test_that("combine_supp works with different IDVARVAL classes", {