diff --git a/R/run_ML.R b/R/run_ML.R index bb9e534..e743463 100644 --- a/R/run_ML.R +++ b/R/run_ML.R @@ -201,9 +201,17 @@ createMLinputList <- function(path, path <- normalizePath(path) -# if (isTRUE(LOO) && (is.null(stratify_by) || !(stratify_by %in% c("year", "country")))) { - # stop("For Leave-One-Out (LOO) models, stratify_by must be 'year' or 'country'.") - # } + # Leave-one-drug-out cross-testing (LOO + cross_test) is a separate mode + # that doesn't stratify by year/country - see the `cross_test && LOO` + # branch below. Stratified LOO (leave-one-year/country-out) always needs + # stratify_by, whether or not it's also cross-tested. + if (isTRUE(LOO) && !isTRUE(cross_test) && + (is.null(stratify_by) || !(stratify_by %in% c("year", "country")))) { + stop( + "For Leave-One-Out (LOO) models without cross-testing, ", + "stratify_by must be 'year' or 'country'." + ) + } if (isTRUE(MDR) && (!is.null(stratify_by) || LOO || cross_test)) { stop("MDR can only run when stratify_by = NULL, LOO = FALSE, cross_test = FALSE.") @@ -579,7 +587,9 @@ parsed_drugs <- parsed |> out_top = paths$ML_top_features, out_models = paths$ML_models, out_pred = paths$ML_prediction - ) + ) + + return(out) } # LOO requires special directory structure resolution test_path <- file.path(path, stringr::str_remove(basename(paths$matrix_path), "^LOO_")) @@ -1002,7 +1012,12 @@ runMLmodels <- function(path, MDR = FALSE, cross_test = cross_test ) - + + if (nrow(files) == 0) { + message("No files found to process. Exiting.") + return(invisible(NULL)) + } + .findNonRanPrefixes <- function(files, seed, shuffle_labels = FALSE) { diff --git a/tests/testthat/test-run-ml-models.R b/tests/testthat/test-run-ml-models.R new file mode 100644 index 0000000..bfe0080 --- /dev/null +++ b/tests/testthat/test-run-ml-models.R @@ -0,0 +1,58 @@ +# Regression tests for control-flow bugs in +# createMLinputList()/runMLmodels(). + +test_that("runMLmodels() exits cleanly instead of crashing on no files", { + tmp <- withr::local_tempdir() + # No matrix parquet files are created, so createMLinputList() returns an + # empty tibble. runMLmodels() must message and return early instead of + # trying to use the empty result (which previously crashed with + # "a character vector argument expected"). + result <- NULL + expect_message( + result <- runMLmodels( + path = tmp, stratify_by = NULL, LOO = FALSE, cross_test = FALSE + ), + "No files found" + ) + expect_null(result) +}) + +test_that("createMLinputList() rejects LOO w/o year/country (no cross-test)", { + tmp <- withr::local_tempdir() + expect_error( + createMLinputList(tmp, LOO = TRUE, cross_test = FALSE, stratify_by = NULL), + "stratify_by must be" + ) +}) + +test_that("createMLinputList() allows LOO+cross-test with stratify_by = NULL", { + # LOO + cross_test with stratify_by = NULL is a distinct mode + # (leave-one-drug-out cross-testing, "Case A" in the cross_test && LOO + # branch) and is intentionally exempt from the year/country requirement + # above. + tmp <- withr::local_tempdir() + paths <- createMLResultDir( + tmp, + stratify_by = NULL, LOO = TRUE, cross_test = TRUE, MDR = FALSE + ) + + # A single matrix filename that satisfies both the general filename parser + # and the LOO-specific parser used inside Case A. File content is + # irrelevant - createMLinputList() only inspects filenames. + file.create(file.path( + paths$matrix_path, + "Csp_drug_leaveout_leaveout_genes_binary_sparse.parquet" + )) + + out <- createMLinputList( + tmp, + stratify_by = NULL, LOO = TRUE, cross_test = TRUE, MDR = FALSE + ) + + # Before the return() fix, Case A built the right result but never + # returned it, so execution fell through into the stratify_by != NULL + # branch, which self-joins the single file against itself and always + # filters it out (ref_file != test_file), silently returning 0 rows. + expect_equal(nrow(out), 1) + expect_true(grepl("_drug_leaveout_", out$output_prefix)) +})