From 70578a59ba3f7a4c13e8f3aeac685866e15afc03 Mon Sep 17 00:00:00 2001 From: Bill Denney Date: Fri, 12 Apr 2024 13:34:31 -0400 Subject: [PATCH 1/5] Remove files from CRAN submission --- CRAN-SUBMISSION | 3 --- DESCRIPTION | 4 ++-- cran-comments.md | 8 -------- 3 files changed, 2 insertions(+), 13 deletions(-) delete mode 100644 CRAN-SUBMISSION delete mode 100644 cran-comments.md diff --git a/CRAN-SUBMISSION b/CRAN-SUBMISSION deleted file mode 100644 index ff7c718..0000000 --- a/CRAN-SUBMISSION +++ /dev/null @@ -1,3 +0,0 @@ -Version: 0.1.4 -Date: 2023-02-13 19:59:37 UTC -SHA: 0eb36c9f9282c3af1ac8d2d2dcaca1a4a5e27f9e diff --git a/DESCRIPTION b/DESCRIPTION index 30b7b12..7d8b598 100644 --- a/DESCRIPTION +++ b/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), diff --git a/cran-comments.md b/cran-comments.md deleted file mode 100644 index 3dd7df1..0000000 --- a/cran-comments.md +++ /dev/null @@ -1,8 +0,0 @@ -## R CMD check results - -There were no ERRORs or WARNINGs. -This was archived because a dependency came off cran.That has now been corrected and this package has been up-versioned - - -## Downstream dependencies -There are currently no downstream dependencies for this package From bfe44226f84b8fad9424452f5ea8e0f333128326 Mon Sep 17 00:00:00 2001 From: Bill Denney Date: Fri, 12 Apr 2024 13:38:09 -0400 Subject: [PATCH 2/5] Update CI to current versions --- .github/workflows/R-CMD-check.yaml | 3 ++- .github/workflows/pkgdown.yaml | 6 ++++-- .github/workflows/test-coverage.yaml | 8 ++++---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index a3ac618..14159b7 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.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")' diff --git a/.github/workflows/pkgdown.yaml b/.github/workflows/pkgdown.yaml index 087f0b0..a7276e8 100644 --- a/.github/workflows/pkgdown.yaml +++ b/.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 diff --git a/.github/workflows/test-coverage.yaml b/.github/workflows/test-coverage.yaml index 2c5bb50..21b8a93 100644 --- a/.github/workflows/test-coverage.yaml +++ b/.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,7 +31,7 @@ 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} @@ -39,12 +39,12 @@ jobs: 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 From 0b8eb98bf26c222230577cbcf8a43c6e7ad3a2f8 Mon Sep 17 00:00:00 2001 From: Bill Denney Date: Fri, 12 Apr 2024 13:45:23 -0400 Subject: [PATCH 3/5] Add failing test --- tests/testthat/test-supp.R | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-supp.R b/tests/testthat/test-supp.R index 4facfb3..84a5b73 100644 --- a/tests/testthat/test-supp.R +++ b/tests/testthat/test-supp.R @@ -217,6 +217,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 data, returning original data unchanged" + ) + expect_equal(result, safetyData::sdtm_ae) +}) From f0fe258a87469da86b94e8e2223d8f39ff647dc6 Mon Sep 17 00:00:00 2001 From: Bill Denney Date: Fri, 12 Apr 2024 13:48:28 -0400 Subject: [PATCH 4/5] Allow zero-row supp data in `combine_supp()` --- NEWS.md | 3 +++ R/supp.R | 4 ++++ tests/testthat/test-supp.R | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 6c9d5ae..40d8329 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,6 @@ +# metatools 0.1.5.9000 +* 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 diff --git a/R/supp.R b/R/supp.R index 29e5271..5a046d4 100644 --- a/R/supp.R +++ b/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") diff --git a/tests/testthat/test-supp.R b/tests/testthat/test-supp.R index 84a5b73..01b16cc 100644 --- a/tests/testthat/test-supp.R +++ b/tests/testthat/test-supp.R @@ -222,7 +222,7 @@ test_that("Floating point correction works", { 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 data, returning original data unchanged" + regexp = "Zero rows in supp, returning original dataset unchanged" ) expect_equal(result, safetyData::sdtm_ae) }) From 87b593a25b3abb876a0324193f7a39de61ff2c4c Mon Sep 17 00:00:00 2001 From: Bill Denney Date: Fri, 12 Apr 2024 16:36:57 -0400 Subject: [PATCH 5/5] `combine_supp()` requires that the QNAM columns are not in the source dataset --- NEWS.md | 1 + R/supp.R | 11 +++++++++-- tests/testthat/test-supp.R | 13 +++++-------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/NEWS.md b/NEWS.md index 40d8329..fbb28e8 100644 --- a/NEWS.md +++ b/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 diff --git a/R/supp.R b/R/supp.R index 5a046d4..85bad52 100644 --- a/R/supp.R +++ b/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 diff --git a/tests/testthat/test-supp.R b/tests/testthat/test-supp.R index 01b16cc..12fbf93 100644 --- a/tests/testthat/test-supp.R +++ b/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", {