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

.github/workflows/R-CMD-check.yaml

@@ -29,7 +29,7 @@ jobs:
       R_KEEP_PKG_SOURCE: yes
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - uses: r-lib/actions/setup-pandoc@v2
 
@@ -47,3 +47,4 @@ jobs:
       - uses: r-lib/actions/check-r-package@v2
         with:
           upload-snapshots: true
+          build_args: 'c("--no-manual","--compact-vignettes=gs+qpdf")'

.github/workflows/pkgdown.yaml

@@ -19,8 +19,10 @@ jobs:
       group: pkgdown-${{ github.event_name != 'pull_request' || github.run_id }}
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+    permissions:
+      contents: write
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - uses: r-lib/actions/setup-pandoc@v2
 
@@ -39,7 +41,7 @@ jobs:
 
       - name: Deploy to GitHub pages 🚀
         if: github.event_name != 'pull_request'
-        uses: JamesIves/github-pages-deploy-action@v4.4.1
+        uses: JamesIves/github-pages-deploy-action@v4.5.0
         with:
           clean: false
           branch: gh-pages

.github/workflows/test-coverage.yaml

@@ -15,7 +15,7 @@ jobs:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - uses: r-lib/actions/setup-r@v2
         with:
@@ -31,20 +31,20 @@ jobs:
           covr::codecov(
             quiet = FALSE,
             clean = FALSE,
-            install_path = file.path(Sys.getenv("RUNNER_TEMP"), "package")
+            install_path = file.path(normalizePath(Sys.getenv("RUNNER_TEMP"), winslash = "/"), "package")
           )
         shell: Rscript {0}
 
       - name: Show testthat output
         if: always()
         run: |
           ## --------------------------------------------------------------------
-          find ${{ runner.temp }}/package -name 'testthat.Rout*' -exec cat '{}' \; || true
+          find '${{ runner.temp }}/package' -name 'testthat.Rout*' -exec cat '{}' \; || true
         shell: bash
 
       - name: Upload test results
         if: failure()
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: coverage-test-failures
           path: ${{ runner.temp }}/package

DESCRIPTION

@@ -1,7 +1,7 @@
 Package: metatools
 Type: Package
 Title: Enable the Use of 'metacore' to Help Create and Check Dataset
-Version: 0.1.5
+Version: 0.1.5.9000
 Authors@R: c(
          person(given = "Christina",
              family = "Fillmore",
@@ -21,7 +21,7 @@ Description: Uses the metadata information stored in 'metacore' objects to check
 License: MIT + file LICENSE
 Encoding: UTF-8
 LazyData: true
-RoxygenNote: 7.2.3
+RoxygenNote: 7.3.1
 Imports: 
     dplyr,
     metacore (>= 0.0.4),

NEWS.md

@@ -1,3 +1,7 @@
+# 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
 * correct bug with `combine_supp()` when the data and the supp have white space. Now it will be trimmed before attempting to merge  
 * Updates made to work with the newest version of dplyr 

R/supp.R

@@ -137,6 +137,10 @@ combine_supp <- function(dataset, supp){
    if(!is.data.frame(dataset) | !is.data.frame(supp)){
       stop("You must supply a domain and supplemental dataset", call. = FALSE)
    }
+  if (nrow(supp) == 0) {
+    warning("Zero rows in supp, returning original dataset unchanged")
+    return(dataset)
+  }
    supp_cols <- c("STUDYID", "RDOMAIN", "USUBJID", "IDVAR", "IDVARVAL",
                   "QNAM", "QLABEL", "QVAL", "QORIG")
    maybe <- c("QEVAL")
@@ -152,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", {
@@ -217,6 +214,12 @@ test_that("Floating point correction works", {
       select(USUBJID, AESEQ = IDVARVAL, AETRTEM = QVAL) %>%
       arrange(USUBJID, AESEQ)
    expect_equal(combo_ae, supp_check)
-   })
-
+})
 
+test_that("zero-row supp returns data unchanged with a warning (#45)", {
+  expect_warning(
+    result <- combine_supp(safetyData::sdtm_ae, safetyData::sdtm_suppae[0,]),
+    regexp = "Zero rows in supp, returning original dataset unchanged"
+  )
+  expect_equal(result, safetyData::sdtm_ae)
+})