From f489be66559f461e356e1d9ac4817ddc3969f1a2 Mon Sep 17 00:00:00 2001 From: Abhirupa Ghosh <100681585+AbhirupaGhosh@users.noreply.github.com> Date: Wed, 24 Jun 2026 15:25:05 -0600 Subject: [PATCH 1/5] Enhance retrieveMetadata() with genome list file input option Updated retrieveMetadata() to allow reading from a specified file if provided. Added checks for file existence and trimmed whitespace from genome IDs. This is the only defining step for the restricted list of genomes that the user is willing to use to build a model. An example application is benchmarking using specific genomes. --- R/data_curation.R | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/R/data_curation.R b/R/data_curation.R index b58cf3c..413b25d 100644 --- a/R/data_curation.R +++ b/R/data_curation.R @@ -912,6 +912,7 @@ #' - table_name: "metadata" #' @export retrieveMetadata <- function(user_bacs, + genome_id_file = NULL, filter_type = "AMR", base_dir = ".", abx = "All", @@ -921,10 +922,37 @@ retrieveMetadata <- function(user_bacs, base_dir <- normalizePath(base_dir, mustWork = FALSE) if (isTRUE(verbose)) message("Resolving genome IDs for user inputs.") - genome_ids <- .retrieveQueryIDs( - base_dir = base_dir, user_bacs = user_bacs, - overwrite = overwrite, verbose = verbose - ) + # ------------------------------- + # GENOME ID RESOLUTION (UPDATED) + # ------------------------------- + if (!is.null(genome_id_file)) { + + if (!file.exists(genome_id_file)) { + stop("Provided genome_id_file does not exist.") + } + + if (isTRUE(verbose)) { + message("Using genome IDs from file: ", genome_id_file) + } + + genome_ids <- readLines(genome_id_file, warn = FALSE) + genome_ids <- trimws(genome_ids) + genome_ids <- genome_ids[genome_ids != ""] + + } else { + + if (isTRUE(verbose)) message("Resolving genome IDs for user inputs.") + + genome_ids <- .retrieveQueryIDs( + base_dir = base_dir, + user_bacs = user_bacs, + overwrite = overwrite, + verbose = verbose + ) + } + + genome_ids <- unique(as.character(genome_ids)) + if (length(genome_ids) == 0) { message("No genome IDs available for the specified inputs.") return(NULL) From 05f05220856cc07e384212a85a8e696b80da5f0e Mon Sep 17 00:00:00 2001 From: AbhirupaGhosh Date: Wed, 24 Jun 2026 21:26:20 +0000 Subject: [PATCH 2/5] Style code (GHA) --- R/data_curation.R | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/R/data_curation.R b/R/data_curation.R index 413b25d..21b6786 100644 --- a/R/data_curation.R +++ b/R/data_curation.R @@ -922,11 +922,10 @@ retrieveMetadata <- function(user_bacs, base_dir <- normalizePath(base_dir, mustWork = FALSE) if (isTRUE(verbose)) message("Resolving genome IDs for user inputs.") - # ------------------------------- + # ------------------------------- # GENOME ID RESOLUTION (UPDATED) # ------------------------------- if (!is.null(genome_id_file)) { - if (!file.exists(genome_id_file)) { stop("Provided genome_id_file does not exist.") } @@ -938,9 +937,7 @@ retrieveMetadata <- function(user_bacs, genome_ids <- readLines(genome_id_file, warn = FALSE) genome_ids <- trimws(genome_ids) genome_ids <- genome_ids[genome_ids != ""] - } else { - if (isTRUE(verbose)) message("Resolving genome IDs for user inputs.") genome_ids <- .retrieveQueryIDs( @@ -952,7 +949,7 @@ retrieveMetadata <- function(user_bacs, } genome_ids <- unique(as.character(genome_ids)) - + if (length(genome_ids) == 0) { message("No genome IDs available for the specified inputs.") return(NULL) From aa2ace5a8f88eccd5362441e0c7ab539a62261b8 Mon Sep 17 00:00:00 2001 From: Abhirupa Ghosh <100681585+AbhirupaGhosh@users.noreply.github.com> Date: Wed, 24 Jun 2026 15:30:11 -0600 Subject: [PATCH 3/5] Fix genome_id_file parameter in prepareGenomes Updated the 'prepareGenomes' function to use 'genome_id_file' parameter correctly. --- R/data_curation.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/data_curation.R b/R/data_curation.R index 21b6786..aab8022 100644 --- a/R/data_curation.R +++ b/R/data_curation.R @@ -1776,6 +1776,7 @@ genomeList <- function(base_dir = ".", #' #' @export prepareGenomes <- function(user_bacs, + genome_id_file = NULL, base_dir = ".", method = c("ftp", "cli"), overwrite = FALSE, @@ -1790,6 +1791,7 @@ prepareGenomes <- function(user_bacs, if (isTRUE(verbose)) message("Step 0: Building AMR metadata (retrieveMetadata)") invisible(retrieveMetadata( user_bacs = user_bacs, + genome_id_file = NULL, filter_type = "AMR", base_dir = base_dir, abx = "All", From 815c7b95e149792a1f5769d59da98282f1667628 Mon Sep 17 00:00:00 2001 From: Emily Boyer Date: Fri, 10 Jul 2026 07:22:32 -0600 Subject: [PATCH 4/5] Pass genome_id_file through prepareGenomes() to retrieveMetadata() prepareGenomes() accepted a genome_id_file argument but hardcoded NULL in its internal retrieveMetadata() call, silently ignoring the file. Pass it through so the restricted genome list flows through the pipeline. Also document the new parameter and drop a duplicate log message. --- R/data_curation.R | 12 ++++++++++-- man/prepareGenomes.Rd | 6 ++++++ man/retrieveMetadata.Rd | 7 +++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/R/data_curation.R b/R/data_curation.R index aab8022..f056c37 100644 --- a/R/data_curation.R +++ b/R/data_curation.R @@ -900,6 +900,11 @@ #' - metadata (join on genome IDs returned by BV-BRC) #' #' @param user_bacs Character vector. Mixed taxon IDs and/or species strings (used for naming). +#' @param genome_id_file Character or NULL. Optional path to a file listing genome +#' IDs (one per line). If provided, genome IDs are read from this file instead of +#' being resolved from `user_bacs`; blank lines and surrounding whitespace are +#' ignored. `user_bacs` is still required for naming the per-selection DuckDB. +#' Default NULL. #' @param filter_type Character. "AMR" or "microTraits". Default "AMR". #' @param base_dir Character. Project root. Default "results/" in legacy scripts; now default ".". #' @param abx Character or vector. Antibiotic filter. "All" for all antibiotics, else names. @@ -921,7 +926,6 @@ retrieveMetadata <- function(user_bacs, verbose = TRUE) { base_dir <- normalizePath(base_dir, mustWork = FALSE) - if (isTRUE(verbose)) message("Resolving genome IDs for user inputs.") # ------------------------------- # GENOME ID RESOLUTION (UPDATED) # ------------------------------- @@ -1760,6 +1764,10 @@ genomeList <- function(base_dir = ".", #' #' @param user_bacs Character vector. Species and/or taxon IDs (e.g. #' `c("Shigella flexneri", "623")`). +#' @param genome_id_file Character or NULL. Optional path to a file listing genome +#' IDs (one per line), passed through to `retrieveMetadata()`. If provided, the +#' metadata step is restricted to these genome IDs instead of resolving them from +#' `user_bacs`. Default NULL. #' @param base_dir Character. Project root directory. Default `"."`. #' @param method Character. Download method passed to `retrieveGenomes()`. #' `"ftp"` (default) or `"cli"`. @@ -1791,7 +1799,7 @@ prepareGenomes <- function(user_bacs, if (isTRUE(verbose)) message("Step 0: Building AMR metadata (retrieveMetadata)") invisible(retrieveMetadata( user_bacs = user_bacs, - genome_id_file = NULL, + genome_id_file = genome_id_file, filter_type = "AMR", base_dir = base_dir, abx = "All", diff --git a/man/prepareGenomes.Rd b/man/prepareGenomes.Rd index a764058..29edd07 100644 --- a/man/prepareGenomes.Rd +++ b/man/prepareGenomes.Rd @@ -6,6 +6,7 @@ \usage{ prepareGenomes( user_bacs, + genome_id_file = NULL, base_dir = ".", method = c("ftp", "cli"), overwrite = FALSE, @@ -17,6 +18,11 @@ prepareGenomes( \item{user_bacs}{Character vector. Species and/or taxon IDs (e.g. \code{c("Shigella flexneri", "623")}).} +\item{genome_id_file}{Character or NULL. Optional path to a file listing genome +IDs (one per line), passed through to \code{retrieveMetadata()}. If provided, the +metadata step is restricted to these genome IDs instead of resolving them from +\code{user_bacs}. Default NULL.} + \item{base_dir}{Character. Project root directory. Default \code{"."}.} \item{method}{Character. Download method passed to \code{retrieveGenomes()}. diff --git a/man/retrieveMetadata.Rd b/man/retrieveMetadata.Rd index 6c92a36..bed8547 100644 --- a/man/retrieveMetadata.Rd +++ b/man/retrieveMetadata.Rd @@ -6,6 +6,7 @@ \usage{ retrieveMetadata( user_bacs, + genome_id_file = NULL, filter_type = "AMR", base_dir = ".", abx = "All", @@ -17,6 +18,12 @@ retrieveMetadata( \arguments{ \item{user_bacs}{Character vector. Mixed taxon IDs and/or species strings (used for naming).} +\item{genome_id_file}{Character or NULL. Optional path to a file listing genome +IDs (one per line). If provided, genome IDs are read from this file instead of +being resolved from \code{user_bacs}; blank lines and surrounding whitespace are +ignored. \code{user_bacs} is still required for naming the per-selection DuckDB. +Default NULL.} + \item{filter_type}{Character. "AMR" or "microTraits". Default "AMR".} \item{base_dir}{Character. Project root. Default "results/" in legacy scripts; now default ".".} From cc23cf86aa51b552c4ae1a0f830aa2ef8eaa5e68 Mon Sep 17 00:00:00 2001 From: Emily Boyer Date: Fri, 10 Jul 2026 07:44:17 -0600 Subject: [PATCH 5/5] Add regression test for genome_id_file pass-through Covers PR #29: prepareGenomes() must forward genome_id_file to retrieveMetadata(), the missing-file guard, and ID-line parsing. Docker-free (mocked), so it runs in CI. --- tests/testthat/test-prepareGenomes.R | 62 ++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 tests/testthat/test-prepareGenomes.R diff --git a/tests/testthat/test-prepareGenomes.R b/tests/testthat/test-prepareGenomes.R new file mode 100644 index 0000000..a230d49 --- /dev/null +++ b/tests/testthat/test-prepareGenomes.R @@ -0,0 +1,62 @@ +# Regression tests for genome_id_file handling. +# +# PR #29: prepareGenomes() accepted a genome_id_file argument but hardcoded +# genome_id_file = NULL in its internal retrieveMetadata() call, so a +# user-supplied genome list was silently ignored via the wrapper (it worked +# only when calling retrieveMetadata() directly). + +test_that("prepareGenomes() forwards genome_id_file to retrieveMetadata()", { + captured <- new.env() + gfile <- tempfile(fileext = ".txt") + writeLines(c("511145.12", "83333.111"), gfile) + on.exit(unlink(gfile), add = TRUE) + + # Stub everything prepareGenomes() touches so we isolate the pass-through. + # retrieveMetadata() records the genome_id_file it receives; .filterGenomes() + # returns NULL so prepareGenomes() exits cleanly right after. + local_mocked_bindings( + .ensure_bvbrc_cache = function(...) invisible(NULL), + retrieveMetadata = function(..., genome_id_file = NULL) { + captured$gid <- genome_id_file + invisible(NULL) + }, + .filterGenomes = function(...) NULL, + .package = "amRdata" + ) + + res <- prepareGenomes( + user_bacs = "Shigella flexneri", + genome_id_file = gfile, + verbose = FALSE + ) + + expect_null(res) + # Before the fix this was NULL (the bug); now it is the file path. + expect_identical(captured$gid, gfile) +}) + +test_that("retrieveMetadata() errors on a missing genome_id_file", { + missing <- file.path(tempdir(), "definitely_not_here_9f8a.txt") + expect_error( + retrieveMetadata( + user_bacs = "x", + genome_id_file = missing, + verbose = FALSE + ), + "does not exist" + ) +}) + +test_that("genome_id_file parsing trims whitespace and drops blank lines", { + # Mirrors the readLines/trimws/drop-empty/unique logic in retrieveMetadata(). + gfile <- tempfile(fileext = ".txt") + writeLines(c("511145.12", " 83333.111 ", "", " ", "511145.12"), gfile) + on.exit(unlink(gfile), add = TRUE) + + ids <- readLines(gfile, warn = FALSE) + ids <- trimws(ids) + ids <- ids[ids != ""] + ids <- unique(as.character(ids)) + + expect_identical(ids, c("511145.12", "83333.111")) +})