Skip to content

Commit 50d1129

Browse files
committed
Enhance shard validation to support mixed numeric types and add comprehensive tests for schema consistency
1 parent b01a222 commit 50d1129

2 files changed

Lines changed: 69 additions & 2 deletions

File tree

‎R/shards.R‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,11 +251,16 @@ create_phewas_manifest <- function(config, path = NULL, write = TRUE) {
251251
if (!length(artifacts)) {
252252
stop("No shard artifacts were supplied.", call. = FALSE)
253253
}
254+
column_type <- function(column) {
255+
type <- typeof(column)
256+
# Numeric storage can vary by shard; rbindlist promotes integers to doubles.
257+
if (type %in% c("integer", "double")) "numeric" else type
258+
}
254259
reference_names <- names(artifacts[[1L]]$results)
255-
reference_types <- vapply(artifacts[[1L]]$results, typeof, character(1L))
260+
reference_types <- vapply(artifacts[[1L]]$results, column_type, character(1L))
256261
for (index in seq_along(artifacts)[-1L]) {
257262
observed_names <- names(artifacts[[index]]$results)
258-
observed_types <- vapply(artifacts[[index]]$results, typeof, character(1L))
263+
observed_types <- vapply(artifacts[[index]]$results, column_type, character(1L))
259264
if (!identical(observed_names, reference_names) ||
260265
!identical(observed_types, reference_types)) {
261266
stop("Shard result schemas are inconsistent.", call. = FALSE)

‎tests/testthat/test-config-shards.R‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,68 @@ test_that("combined results are validated and the TSV is repaired on reuse", {
727727
expect_identical(readLines(tsv_path, warn = FALSE), expected_tsv)
728728
})
729729

730+
test_that("shard schemas accept integer and double numeric columns", {
731+
artifacts <- list(
732+
list(results = data.frame(df = c(1L, NA_integer_), n = c(20, 30))),
733+
list(results = data.frame(df = c(2.5, NA_real_), n = c(40L, 50L)))
734+
)
735+
736+
expect_true(phewasFlow:::.pf_validate_shard_set(artifacts))
737+
expect_true(phewasFlow:::.pf_validate_shard_set(rev(artifacts)))
738+
})
739+
740+
test_that("shard schemas still reject incompatible types, names, and order", {
741+
reference <- list(results = data.frame(df = 1L, n = 20L))
742+
for (value in list("1", TRUE, 1 + 0i, list(1L))) {
743+
altered <- reference
744+
altered$results$df <- value
745+
expect_error(
746+
phewasFlow:::.pf_validate_shard_set(list(reference, altered)),
747+
"Shard result schemas are inconsistent"
748+
)
749+
}
750+
for (columns in list("df", c("n", "df"))) {
751+
altered <- list(results = reference$results[, columns, drop = FALSE])
752+
expect_error(
753+
phewasFlow:::.pf_validate_shard_set(list(reference, altered)),
754+
"Shard result schemas are inconsistent"
755+
)
756+
}
757+
altered <- reference
758+
names(altered$results)[[1L]] <- "degrees_of_freedom"
759+
expect_error(
760+
phewasFlow:::.pf_validate_shard_set(list(reference, altered)),
761+
"Shard result schemas are inconsistent"
762+
)
763+
})
764+
765+
test_that("mixed integer and double df shards combine without changing values", {
766+
directory <- withr::local_tempdir()
767+
fixture <- complete_shard_fixture(directory)
768+
artifacts <- lapply(fixture$shards, readRDS)
769+
770+
for (double_shard in seq_along(artifacts)) {
771+
for (index in seq_along(artifacts)) {
772+
artifact <- data.table::copy(artifacts[[index]])
773+
artifact$results$df <- if (index == double_shard) {
774+
as.double(artifact$results$df)
775+
} else {
776+
as.integer(artifact$results$df)
777+
}
778+
saveRDS(artifact, fixture$shards[[index]], version = 3L)
779+
}
780+
781+
result <- combine_phewas_shards(fixture$config, overwrite = TRUE)
782+
expect_identical(result$df, as.double(fixture$result$df))
783+
expect_equal(result, fixture$result, ignore_attr = TRUE)
784+
expect_equal(read_phewas_bundle(fixture$output), result, ignore_attr = TRUE)
785+
expect_equal(
786+
suppressMessages(combine_phewas_shards(fixture$config)),
787+
result, ignore_attr = TRUE
788+
)
789+
}
790+
})
791+
730792
test_that("readable shard artifacts cannot hide schema or metadata drift", {
731793
directory <- withr::local_tempdir()
732794
path <- make_shard_fixture(directory)

0 commit comments

Comments
 (0)