Skip to content

Commit 9363e99

Browse files
VALIDATOR fails if only formula contrast is specified (#80)
* Work when only using formula and no comparison * Rename formula_vars in accessory.R * Validate contrasts individually and remove use of temporary columns * Add test for fomula contrasts alone
1 parent d2ca950 commit 9363e99

2 files changed

Lines changed: 82 additions & 32 deletions

File tree

R/accessory.R

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,41 +1019,48 @@ read_contrasts <-
10191019
success <- checkListIsSubset(blocking, colnames(samples), "blocking variables", "sample metadata")
10201020
}
10211021

1022-
# Extract design matrix columns from contrasts: the variable column plus any blocking factors.
1023-
design_cols <- unique(na.omit(c(contrasts[[variable_column]], blocking)))
1024-
design_matrix <- samples[, design_cols, drop = FALSE]
1025-
1026-
# Ensure there are no NA values in the design matrix.
1027-
if (any(is.na(design_matrix))) {
1028-
stop("NA values found in one or more design matrix columns.")
1029-
}
1030-
1031-
# Check that the design matrix is full rank.
1032-
mm <- model.matrix(~ . - 1, data = design_matrix)
1033-
if (qr(mm)$rank < ncol(mm)) {
1034-
stop(paste("Design matrix is not full rank.", "Model matrix columns:", paste(colnames(mm), collapse = ", "), "\n"))
1035-
}
1036-
1037-
# Warn about continuous covariates in the design matrix columns.
1038-
for (col in design_cols) {
1039-
if (is.numeric(samples[[col]])) {
1040-
warning(paste("Column", col, "is numeric and may be treated as continuous."))
1022+
# Ensure reference and target are valid for their variable
1023+
for (i in 1:nrow(contrasts)) {
1024+
blocking_vars <- simpleSplit(contrasts[[blocking_column]][i], ";")
1025+
1026+
# Extract design matrix columns from contrasts: the variable column plus any blocking factors.
1027+
# For formula-based contrasts, extract variables from the formula itself.
1028+
formula_vars <- character(0)
1029+
if ("formula" %in% colnames(contrasts) && !is.na(contrasts$formula[i])) {
1030+
formula_vars <- all.vars(as.formula(contrasts$formula[i]))
10411031
}
1042-
}
1043-
1044-
# Check that values in design matrix columns do not contain disallowed special characters.
1045-
for (col in design_cols) {
1046-
vals <- as.character(samples[[col]])
1047-
for (sc in c("/", "\\\\")) { # Default special characters: c("/", "\\\\")
1048-
if (any(grepl(sc, vals))) {
1049-
warning(paste("Column", col, "contains special character", sc,
1050-
"which may cause issues downstream."))
1032+
1033+
design_cols <- unique(na.omit(c(contrasts[[variable_column]][i], blocking_vars, formula_vars)))
1034+
design_matrix <- samples[, design_cols, drop = FALSE]
1035+
1036+
# Ensure there are no NA values in the design matrix.
1037+
if (any(is.na(design_matrix))) {
1038+
stop("NA values found in one or more design matrix columns.")
1039+
}
1040+
1041+
# Check that the design matrix is full rank.
1042+
mm <- model.matrix(~ . - 1, data = design_matrix)
1043+
if (qr(mm)$rank < ncol(mm)) {
1044+
stop(paste("Design matrix is not full rank.", "Model matrix columns:", paste(colnames(mm), collapse = ", "), "\n"))
1045+
}
1046+
1047+
# Warn about continuous covariates in the design matrix columns.
1048+
for (col in design_cols) {
1049+
if (is.numeric(samples[[col]])) {
1050+
warning(paste("Column", col, "is numeric and may be treated as continuous."))
1051+
}
1052+
}
1053+
1054+
# Check that values in design matrix columns do not contain disallowed special characters.
1055+
for (col in design_cols) {
1056+
vals <- as.character(samples[[col]])
1057+
for (sc in c("/", "\\\\")) { # Default special characters: c("/", "\\\\")
1058+
if (any(grepl(sc, vals))) {
1059+
warning(paste("Column", col, "contains special character", sc,
1060+
"which may cause issues downstream."))
1061+
}
10511062
}
10521063
}
1053-
}
1054-
1055-
# Ensure reference and target are valid for their variable
1056-
for (i in 1:nrow(contrasts)) {
10571064
var <- contrasts[i, variable_column]
10581065
ref <- contrasts[i, reference_column]
10591066
tgt <- contrasts[i, target_column]

tests/testthat/test-accessory.R

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,48 @@ contrasts:
8181
expect_equal(contrasts$make_contrasts_str[4], "genotypeWT.treatmentTreated")
8282
expect_equal(contrasts$make_contrasts_str[5], "genotypeWT.treatmentTreated.time")
8383

84+
unlink(yaml_file)
85+
})
86+
87+
# read_contrasts() using only formula based contrasts
88+
89+
test_that("read_contrasts parses YAML correctly using only formula based contrasts", {
90+
samples <- data.frame(
91+
sample = c("Sample1", "Sample7", "Sample13", "Sample19", "Sample16"),
92+
genotype = c("WT", "WT", "KO", "KO", "KO"),
93+
treatment = c("Control", "Treated", "Control", "Treated", "Control"),
94+
time = c(1, 1, 1, 1, 16),
95+
batch = c("b1", "b1", "b1", "b1", "b3"),
96+
stringsAsFactors = FALSE
97+
)
98+
99+
yaml_content <- "
100+
contrasts:
101+
- id: treatment_plus_genotype
102+
formula: \"~ treatment + genotype\"
103+
make_contrasts_str: \"treatmentTreated\"
104+
- id: interaction_genotype_treatment
105+
formula: \"~ genotype * treatment\"
106+
make_contrasts_str: \"genotypeWT.treatmentTreated\"
107+
- id: full_model_with_interactions
108+
formula: \"~ genotype * treatment * time\"
109+
make_contrasts_str: \"genotypeWT.treatmentTreated.time\"
110+
"
111+
112+
yaml_file <- tempfile(fileext = ".yaml")
113+
writeLines(yaml_content, yaml_file)
114+
115+
contrasts <- read_contrasts(yaml_file, samples)
116+
117+
# Test basic structure
118+
expect_true(is.data.frame(contrasts))
119+
expect_equal(nrow(contrasts), 3)
120+
expect_true(all(c("id", "variable", "reference", "target", "blocking", "formula", "make_contrasts_str") %in% colnames(contrasts)))
121+
122+
# Test specific rows
123+
expect_equal(contrasts$formula[1], "~ treatment + genotype")
124+
expect_equal(contrasts$make_contrasts_str[2], "genotypeWT.treatmentTreated")
125+
expect_equal(contrasts$make_contrasts_str[3], "genotypeWT.treatmentTreated.time")
126+
84127
unlink(yaml_file)
85128
})

0 commit comments

Comments
 (0)