From 0ae50f9a359ca799d2071c0fd1ee393f7e952798 Mon Sep 17 00:00:00 2001 From: Natalia Garcia Date: Wed, 27 May 2026 08:10:04 +0000 Subject: [PATCH] Check is a valid sample sheet use the extensive validation [CW-7210] --- .gitlab-ci.yml | 9 +- bin/workflow_glue/check_experiment_design.py | 126 ----- .../common/test_check_experiment_design.py | 72 --- .../tests/common/test_check_sample_sheet.py | 481 ++++++++++++++++++ .../wfg_helpers/validators/wf.py | 231 +++++++++ bin/workflow_glue_r/R/bambu.R | 29 +- bin/workflow_glue_r/R/common.R | 12 + bin/workflow_glue_r/R/de_analysis.R | 49 +- .../tests/testthat/test_bambu.R | 35 -- .../tests/testthat/test_de_analysis.R | 97 +--- main.nf | 18 + subworkflows/differential_expression.nf | 26 +- 12 files changed, 759 insertions(+), 426 deletions(-) delete mode 100644 bin/workflow_glue/check_experiment_design.py delete mode 100644 bin/workflow_glue/tests/common/test_check_experiment_design.py create mode 100644 bin/workflow_glue/tests/common/test_check_sample_sheet.py create mode 100644 bin/workflow_glue/wfg_helpers/validators/wf.py diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8a7b36b..a0fa539 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -69,7 +69,7 @@ docker-run: - MATRIX_NAME: [ "int_discover_dna", "int_fixed_rna", "int_de_control_vs_control", "smoke_discover", "smoke_fixed", "smoke_direct_rna", "smoke_de", - "mouse_de_0countquant", "mods_bigwig_igv" + "mouse_de_0countquant", "mods_bigwig_igv", "mismatch-sample-alias" ] rules: # NOTE As we're overriding the rules block for the included docker-run @@ -173,6 +173,13 @@ docker-run: test -f ${CI_PROJECT_NAME}/de_analysis/condition_treated_vs_control/results_dge.tsv && test -f ${CI_PROJECT_NAME}/de_analysis/condition_treated_vs_control/results_dtu_transcript.tsv && [ "$(find ${CI_PROJECT_NAME}/samples -type f -name 'gene_counts.tsv' | wc -l)" -eq 4 ] + - if: $MATRIX_NAME == "mismatch-sample-alias" + variables: + NF_BEFORE_SCRIPT: ":" + NF_WORKFLOW_OPTS: "--fastq test_data/smoke/de/barcode01/reads.fastq --sample_sheet test_data/smoke/sample_sheet_de.csv --ref_genome test_data/smoke/reference.fa --ref_annotation test_data/smoke/annotation.gtf" + ASSERT_NEXTFLOW_FAILURE: "1" + AFTER_NEXTFLOW_CMD: > + grep -F "Sample alias 'reads' was not found in the sample_sheet alias column." .nextflow.log # Tests a common error when there are 0 annotation counts for a chunk - if: $MATRIX_NAME == "mouse_de_0countquant" variables: diff --git a/bin/workflow_glue/check_experiment_design.py b/bin/workflow_glue/check_experiment_design.py deleted file mode 100644 index 03b3854..0000000 --- a/bin/workflow_glue/check_experiment_design.py +++ /dev/null @@ -1,126 +0,0 @@ -"""Validate DE/DTU sample sheet settings.""" - -from collections import Counter -import csv -import sys - -from .util import get_named_logger, wf_parser # noqa: ABS101 - - -def _split_covariates(value): - if not value: - return [] - return [part.strip() for part in value.split(",") if part.strip()] - - -def main(args): - """Validate sample sheet content for DE/DTU.""" - logger = get_named_logger("checkDesign") - covariates = _split_covariates(args.covariates) - with open(args.sample_sheet, "r", newline="") as handle: - reader = csv.DictReader(handle) - if reader.fieldnames is None: - sys.exit("Sample sheet is empty.") - fieldnames = set(reader.fieldnames) - required = {"alias", args.condition_column} - missing = sorted(required - fieldnames) - if missing: - sys.exit( - "Sample sheet is missing required columns: " - + ", ".join(missing) - ) - - missing_covariates = [name for name in covariates if name not in fieldnames] - if missing_covariates: - sys.exit( - "Sample sheet is missing requested covariate columns: " - + ", ".join(missing_covariates) - ) - - aliases = [] - levels = Counter() - for row in reader: - alias = row.get("alias", "").strip() - if not alias: - sys.exit("Sample sheet contains a row with an empty alias value.") - aliases.append(alias) - - value = row.get(args.condition_column, "").strip() - if not value: - sys.exit( - f"Sample sheet contains a row with an empty " - f"'{args.condition_column}' value." - ) - levels[value] += 1 - - for covariate in covariates: - if not row.get(covariate, "").strip(): - sys.exit( - f"Sample sheet contains an empty value in covariate column " - f"'{covariate}'." - ) - - duplicate_aliases = [ - alias for alias, count in Counter(aliases).items() if count > 1 - ] - if duplicate_aliases: - sys.exit( - "Sample sheet aliases must be unique. Duplicates: " - + ", ".join(sorted(duplicate_aliases)) - ) - - if len(levels) < 2: - sys.exit( - f"The condition column '{args.condition_column}' must contain at " - "least two levels." - ) - - reference_level = args.reference_level - if not reference_level: - if "control" in levels: - reference_level = "control" - else: - sys.exit( - "Provide --reference_level when the condition column does not " - "contain 'control'." - ) - - if reference_level not in levels: - sys.exit( - f"Reference level '{reference_level}' was absent from " - f"'{args.condition_column}'." - ) - - underpowered = [level for level, count in levels.items() if count < 2] - if underpowered: - sys.exit( - "Each condition level must contain at least two samples. " - "Levels with too few samples: " - + ", ".join(sorted(underpowered)) - ) - - logger.info( - "Validated sample sheet %s using condition column '%s' with reference '%s'.", - args.sample_sheet, - args.condition_column, - reference_level, - ) - - -def argparser(): - """Argument parser for the validation entry point.""" - parser = wf_parser("check_experiment_design") - parser.add_argument("--sample_sheet", required=True, help="Sample sheet CSV.") - parser.add_argument( - "--condition_column", default="condition", - help="Primary biological variable column." - ) - parser.add_argument( - "--covariates", default=None, - help="Comma-separated nuisance covariates." - ) - parser.add_argument( - "--reference_level", default=None, - help="Reference level of the primary condition column." - ) - return parser diff --git a/bin/workflow_glue/tests/common/test_check_experiment_design.py b/bin/workflow_glue/tests/common/test_check_experiment_design.py deleted file mode 100644 index 608bb21..0000000 --- a/bin/workflow_glue/tests/common/test_check_experiment_design.py +++ /dev/null @@ -1,72 +0,0 @@ -"""Tests for DE/DTU design validation.""" - -import pytest -from workflow_glue import check_experiment_design - - -def _write(path, text): - path.write_text(text, encoding="utf-8") - return path - - -def _args(*argv): - return check_experiment_design.argparser().parse_args(list(argv)) - - -def test_split_covariates_trims_and_drops_empty_values(): - """Covariates should be normalised into a clean list.""" - assert check_experiment_design._split_covariates( - " batch, sex ,, site " - ) == ["batch", "sex", "site"] - - -def test_main_accepts_valid_design(tmp_path): - """A balanced two-condition sample sheet should validate cleanly.""" - sample_sheet = _write( - tmp_path / "sample_sheet.csv", - ( - "alias,condition,batch\n" - "control_rep1,control,b1\n" - "control_rep2,control,b2\n" - "treated_rep1,treated,b1\n" - "treated_rep2,treated,b2\n" - ), - ) - - check_experiment_design.main( - _args("--sample_sheet", str(sample_sheet), "--covariates", "batch") - ) - - -def test_main_rejects_duplicate_aliases(tmp_path): - """Duplicate aliases should fail validation.""" - sample_sheet = _write( - tmp_path / "sample_sheet.csv", - ( - "alias,condition\n" - "rep1,control\n" - "rep1,control\n" - "rep3,treated\n" - "rep4,treated\n" - ), - ) - - with pytest.raises(SystemExit, match="aliases must be unique"): - check_experiment_design.main(_args("--sample_sheet", str(sample_sheet))) - - -def test_main_requires_reference_level_when_control_is_absent(tmp_path): - """Non-control condition labels require an explicit reference level.""" - sample_sheet = _write( - tmp_path / "sample_sheet.csv", - ( - "alias,condition\n" - "rep1,baseline\n" - "rep2,baseline\n" - "rep3,treated\n" - "rep4,treated\n" - ), - ) - - with pytest.raises(SystemExit, match="Provide --reference_level"): - check_experiment_design.main(_args("--sample_sheet", str(sample_sheet))) diff --git a/bin/workflow_glue/tests/common/test_check_sample_sheet.py b/bin/workflow_glue/tests/common/test_check_sample_sheet.py new file mode 100644 index 0000000..ff9721e --- /dev/null +++ b/bin/workflow_glue/tests/common/test_check_sample_sheet.py @@ -0,0 +1,481 @@ +"""Test check_sample_sheet.py.""" + +import json + +import pytest +from workflow_glue.wfg_helpers import check_sample_sheet +from workflow_glue.wfg_helpers.validators.wf import _validate_r_formula_names + + +def _run_check_sample_sheet(sample_sheet_path, params_json, capsys): + """Run sample-sheet validation and return captured stdout.""" + args = [str(sample_sheet_path), str(params_json), "--no_barcode"] + parsed_args = check_sample_sheet.argparser().parse_args(args) + + try: + check_sample_sheet.main(parsed_args) + except SystemExit: + pass + + out, _ = capsys.readouterr() + return out + + +@pytest.mark.parametrize( + "names", + [ + [ + "condition", "batch_1", "site.2", + "S01", "a1", "sample_group", + "01", "1batch", "2.site-3" + ], + ], +) +def test_validate_r_formula_names_accepts_valid_names(names): + """R formula names accept alnum-led names with safe punctuation.""" + _validate_r_formula_names(names) + + +@pytest.mark.parametrize( + "names", + [ + ["_condition"], + ["condition 1"], + ["size;batch"], + ["size:batch"], + ["size|batch"], + ["sueño"], + [""] + ], +) +def test_validate_r_formula_names_rejects_invalid_names(names): + """Invalid R formula names should raise a validation error.""" + with pytest.raises(ValueError): + _validate_r_formula_names(names) + + +def test_check_sample_sheet_skips_condition_check_without_condition_column_param( + tmp_path, capsys +): + """Test that condition checks are skipped without the param.""" + # This would fail (as there is no enough replicates) + # but in this case is ok as de_analysis is not enabled + sample_sheet_path = tmp_path / "sample_sheet_condition_counts.csv" + sample_sheet_path.write_text( + "sample_name,condition\n" + "test_name1,condition1\n" + "test_name2,condition2\n" + "test_name3,condition2\n" + ) + + params_json = tmp_path / "params.json" + # condition has a default from the nextflow schema + params_json.write_text( + json.dumps({"de_analysis": False, "condition_column": "condition"}) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out == "" + + +def test_check_sample_sheet_rejects_condition_with_single_sample_when_param_present( + tmp_path, capsys +): + """Test that each condition must have at least two samples when DE is enabled.""" + sample_sheet_path = tmp_path / "sample_sheet_condition_counts.csv" + sample_sheet_path.write_text( + "sample_name,type,condition\n" + "test_name1,test_sample,condition1\n" + "test_name2,positive_control,condition2\n" + "test_name3,negative_control,condition2\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({"de_analysis": True, "condition_column": "condition"}) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out.startswith( + "Condition must have at least 2 samples: condition1 " + "(column: condition)" + ) + + +def test_check_sample_sheet_requires_at_least_two_condition_levels( + tmp_path, capsys +): + """DE-enabled sample sheets must contain at least two condition levels.""" + sample_sheet_path = tmp_path / "sample_sheet_single_condition_level.csv" + sample_sheet_path.write_text( + "sample_name,type,condition\n" + "test_name1,test_sample,condition1\n" + "test_name2,positive_control,condition1\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({"de_analysis": True, "condition_column": "condition"}) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out.startswith( + "Condition column must contain at least 2 condition levels " + "(column: condition)" + ) + + +def test_check_sample_sheet_rejects_missing_condition_values( + tmp_path, capsys +): + """Sample sheets must not contain empty condition cells.""" + sample_sheet_path = tmp_path / "sample_sheet_missing_condition_value.csv" + sample_sheet_path.write_text( + "sample_name,type,condition\n" + "test_name1,test_sample,\n" + "test_name2,positive_control,treated\n" + "test_name3,negative_control,treated\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({"de_analysis": True, "condition_column": "condition"}) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out.startswith( + "Condition column must not contain missing or empty values " + "(column: condition, line: 1)" + ) + + +def test_check_sample_sheet_requires_condition_column_when_param_present( + tmp_path, capsys +): + """Configured condition columns must exist when DE is enabled.""" + sample_sheet_path = tmp_path / "sample_sheet_missing_condition.csv" + sample_sheet_path.write_text( + "sample_name,batch\n" + "test_name1,b1\n" + "test_name2,b2\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({"de_analysis": True, "condition_column": "condition"}) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out.startswith( + "Sample sheet must contain the 'condition' column. " + "(column: condition)" + ) + + +def test_check_sample_sheet_requires_covariate_columns_when_present( + tmp_path, capsys +): + """Check covariate columns exist when provided and DE is enabled.""" + sample_sheet_path = tmp_path / "sample_sheet_missing_covariate.csv" + sample_sheet_path.write_text( + "sample_name,condition\n" + "test_name1,control\n" + "test_name2,control\n" + "test_name3,treated\n" + "test_name4,treated\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({ + "de_analysis": True, + "condition_column": "condition", + "covariates": "batch,site", + }) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out.startswith("Missing covariate columns: batch, site") + + +def test_check_sample_sheet_rejects_missing_covariate_values( + tmp_path, capsys +): + """Configured covariates must have a value in each row when DE is enabled.""" + sample_sheet_path = tmp_path / "sample_sheet_missing_covariate_value.csv" + sample_sheet_path.write_text( + "sample_name,condition,batch,site\n" + "test_name1,control,,s1\n" + "test_name2,control,b1,s2\n" + "test_name3,treated,b2,s1\n" + "test_name4,treated,b2,s2\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({ + "de_analysis": True, + "condition_column": "condition", + "covariates": "batch,site", + }) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out.startswith( + "Covariate column must not contain missing or empty values " + "(column: batch, line: 1)" + ) + + +def test_check_sample_sheet_rejects_unsafe_covariate_values( + tmp_path, capsys +): + """Configured covariate values must use safe contrast/design characters.""" + sample_sheet_path = tmp_path / "sample_sheet_invalid_covariate_value.csv" + sample_sheet_path.write_text( + "sample_name,condition,good-batch,site\n" + "test_name1,control,b1,s1\n" + "test_name2,control,b2,s2\n" + "test_name3,treated,b 1,s1\n" + "test_name4,treated,b2,s2\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({ + "de_analysis": True, + "condition_column": "condition", + "covariates": "good-batch,site", + }) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out.startswith( + "Covariate value names must be safe for R formulas. " + "Invalid names: b 1. Names must start with a letter or number and " + "contain only letters, numbers, underscores, dots, and hyphens. " + "(column: good-batch, line: 3)" + ) + + +def test_check_sample_sheet_rejects_unsafe_column_names( + tmp_path, capsys +): + """Configured DE condition and covariate column names may include hyphens.""" + sample_sheet_path = tmp_path / "sample_sheet_invalid_design_name.csv" + sample_sheet_path.write_text( + "sample_name,condition-column,bad covariate\n" + "test_name1,control,b1\n" + "test_name2,control,b2\n" + "test_name3,treated,b1\n" + "test_name4,treated,b2\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({ + "de_analysis": True, + "condition_column": "condition-column", + "covariates": "bad covariate", + }) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + assert out.startswith( + "Covariate column names must be safe for R formulas. " + "Invalid names: bad covariate. Names must start with a letter or " + "number and contain only letters, numbers, underscores, dots, and " + "hyphens." + ) + + +def test_check_sample_sheet_rejects_unsafe_condition_values( + tmp_path, capsys +): + """Condition values must be safe for contrast output paths.""" + sample_sheet_path = tmp_path / "sample_sheet_invalid_condition_value.csv" + sample_sheet_path.write_text( + "sample_name,type,condition,batch,site\n" + "test_name1,test_sample,control/group,b1,s1\n" + "test_name2,positive_control,control/group,b1,s2\n" + "test_name3,test_sample,treated,b2,s1\n" + "test_name4,negative_control,treated,b2,s2\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({ + "de_analysis": True, + "condition_column": "condition", + "covariates": "batch,site", + }) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out.splitlines() == [ + "Condition value names must be safe for R formulas. " + "Invalid names: control/group. Names must start with a letter or " + "number and contain only letters, numbers, underscores, dots, and " + "hyphens. (column: condition, line: 1)", + "Condition value names must be safe for R formulas. " + "Invalid names: control/group. Names must start with a letter or " + "number and contain only letters, numbers, underscores, dots, and " + "hyphens. (column: condition, line: 2)", + ] + + +def test_check_sample_sheet_accepts_control_as_default_reference_level( + tmp_path, capsys +): + """Control is accepted as the default reference level when present.""" + sample_sheet_path = tmp_path / "sample_sheet_reference_control.csv" + sample_sheet_path.write_text( + "sample_name,type,condition,batch,site\n" + "test_name1,test_sample,control,b1,s1\n" + "test_name2,positive_control,control,b1,s2\n" + "test_name3,test_sample,treated,b2,s1\n" + "test_name4,negative_control,treated,b2,s2\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({ + "de_analysis": True, + "condition_column": "condition", + "covariates": "batch,site", + }) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out == "" + + +def test_check_sample_sheet_requires_reference_level_without_control( + tmp_path, capsys +): + """A non-control design requires an explicit reference level.""" + sample_sheet_path = tmp_path / "sample_sheet_missing_reference.csv" + sample_sheet_path.write_text( + "sample_name,type,condition,batch,site\n" + "test_name1,test_sample,baseline,b1,s1\n" + "test_name2,positive_control,baseline,b1,s2\n" + "test_name3,test_sample,treated,b2,s1\n" + "test_name4,negative_control,treated,b2,s2\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({ + "de_analysis": True, + "condition_column": "condition", + "covariates": "batch,site", + }) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + assert out == ( + "Provide --reference_level when the condition column " + "does not match the default 'control'.\n" + ) + + +def test_check_sample_sheet_accepts_requested_reference_level( + tmp_path, capsys +): + """A configured reference level is accepted when present.""" + sample_sheet_path = tmp_path / "sample_sheet_valid_reference.csv" + sample_sheet_path.write_text( + "sample_name,type,condition,batch,site\n" + "test_name1,test_sample,baseline,b1,s1\n" + "test_name2,positive_control,baseline,b1,s2\n" + "test_name3,test_sample,treated,b2,s1\n" + "test_name4,negative_control,treated,b2,s2\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({ + "de_analysis": True, + "condition_column": "condition", + "covariates": "batch,site", + "reference_level": "baseline", + }) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out == "" + + +def test_check_sample_sheet_rejects_missing_requested_reference_level( + tmp_path, capsys +): + """Configured reference levels must exist in the condition column.""" + sample_sheet_path = tmp_path / "sample_sheet_invalid_reference.csv" + sample_sheet_path.write_text( + "sample_name,type,condition,batch,site\n" + "test_name1,test_sample,baseline,b1,s1\n" + "test_name2,positive_control,baseline,b1,s2\n" + "test_name3,test_sample,treated,b2,s1\n" + "test_name4,negative_control,treated,b2,s2\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({ + "de_analysis": True, + "condition_column": "condition", + "covariates": "batch,site", + "reference_level": "control", + }) + ) + + out = _run_check_sample_sheet(sample_sheet_path, params_json, capsys) + + assert out.startswith(( + "The requested reference level 'control' is not present in " + "the condition column.") + ) + + +def test_check_sample_sheet_skips_de_checks_when_de_analysis_is_disabled( + tmp_path, capsys +): + """DE design columns are optional unless DE analysis is enabled.""" + sample_sheet_path = tmp_path / "sample_sheet_no_condition.csv" + sample_sheet_path.write_text( + "barcode,alias\n" + "barcode01,test_name1\n" + "barcode02,test_name2\n" + ) + + params_json = tmp_path / "params.json" + params_json.write_text( + json.dumps({ + "de_analysis": False, + "condition_column": "condition", + "covariates": "batch", + }) + ) + + args = [str(sample_sheet_path), str(params_json)] + parsed_args = check_sample_sheet.argparser().parse_args(args) + + try: + check_sample_sheet.main(parsed_args) + except SystemExit: + pass + + out, _ = capsys.readouterr() + + assert out == "" diff --git a/bin/workflow_glue/wfg_helpers/validators/wf.py b/bin/workflow_glue/wfg_helpers/validators/wf.py new file mode 100644 index 0000000..7975fab --- /dev/null +++ b/bin/workflow_glue/wfg_helpers/validators/wf.py @@ -0,0 +1,231 @@ +"""Workflow-specific sample sheet validators.""" + +from collections import Counter +import re + +from .default import SampleSheetValidator # noqa: ABS101 + +_R_FORMULA_NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$") + + +def _split_covariates(value): + """Normalise workflow covariates into a clean list.""" + if isinstance(value, str): + return [item.strip() for item in value.split(",") if item.strip()] + + +def _validate_r_formula_names(names, label="Column"): + """Validate names against the shared design-name pattern.""" + empty = names.count("") + invalid = [ + name for name in names + if name != "" and ( + not isinstance(name, str) or _R_FORMULA_NAME_RE.match(name) is None + ) + ] + if empty or invalid: + details = [] + if empty: + details.append('Empty value: ""') + if invalid: + details.append(f"Invalid names: {', '.join(invalid)}") + raise ValueError( + f"{label} names must be safe for R formulas. {'; '.join(details)}. " + "Names must start with a letter or number and contain " + "only letters, numbers, underscores, dots, and hyphens." + ) + + +class Covariates(SampleSheetValidator): + """Validate optional DE/DTU covariate columns from workflow params.""" + + def __init__(self, wf_params, options=None): + """Initialize with workflow parameters.""" + super().__init__(wf_params, options) + if wf_params.get("covariates"): + self.covariates = _split_covariates(wf_params.get("covariates")) + else: + self.covariates = [] + + def on_header(self, header): + """Require configured covariate columns when they are requested.""" + if not self.wf_params.get("de_analysis"): + return + # Check values of condition are safe for R + try: + _validate_r_formula_names( + self.covariates, + label="Covariate column", + ) + except ValueError as exc: + self.log_error(str(exc)) + + missing_covariates = [ + covariate for covariate in self.covariates + if covariate not in header + ] + if missing_covariates: + self.log_error( + f"Missing covariate columns: {', '.join(missing_covariates)}" + ) + + def add_sheet_row(self, row, lineno): + """Require values in requested design columns for each row.""" + if not self.wf_params.get("de_analysis"): + return + + for covariate in self.covariates: + if covariate in row and not row.get(covariate): + self.log_error( + "Covariate column must not contain missing or empty values", + column=covariate, + lineno=lineno, + ) + continue + + if covariate in row: + try: + _validate_r_formula_names( + [row.get(covariate)], + label="Covariate value", + ) + except ValueError as exc: + self.log_error( + str(exc), + column=covariate, + lineno=lineno, + ) + + +class ConditionReplicates(SampleSheetValidator): + """Validate the condition column and check that each level has replicates.""" + + def __init__(self, wf_params, options=None): + """Initialize with workflow parameters.""" + super().__init__(wf_params, options) + self.condition_column = wf_params.get("condition_column") + self.has_condition = False + self.condition_counts = Counter() + + def on_header(self, header): + """Validate and track whether the sample sheet contains a condition column.""" + if not self.wf_params.get("de_analysis"): + return + + try: + _validate_r_formula_names( + [self.condition_column], + label="Design column", + ) + except ValueError as exc: + self.log_error(str(exc)) + + self.has_condition = ( + self.condition_column is not None and self.condition_column in header + ) + + if self.condition_column and self.condition_column not in header: + self.log_error( + f"Sample sheet must contain the '{self.condition_column}' column.", + column=self.condition_column, + ) + + def add_sheet_row(self, row, lineno): + """Validate condition values and count samples for each condition.""" + if not self.has_condition: + return + + condition = row.get(self.condition_column) + if not condition: + self.log_error( + "Condition column must not contain missing or empty values", + column=self.condition_column, + lineno=lineno, + ) + return + # Check values of condition are safe for R + try: + _validate_r_formula_names( + [condition], + label="Condition value", + ) + except ValueError as exc: + self.log_error(str(exc), column=self.condition_column, lineno=lineno) + + self.condition_counts[condition] += 1 + + @property + def is_valid(self): + """Check condition levels and replicates are sufficient for DE.""" + valid = True + if self.has_condition and len(self.condition_counts) < 2: + self.log_error( + "Condition column must contain at least 2 condition levels", + column=self.condition_column, + ) + valid = False + for condition, count in self.condition_counts.items(): + if count < 2: + self.log_error( + f"Condition must have at least 2 samples: {condition}", + column=self.condition_column, + ) + valid = False + return valid and super().is_valid + + +class ReferenceLevel(SampleSheetValidator): + """Validate the DE reference level against observed condition values.""" + + def __init__(self, wf_params, options=None): + """Initialize with workflow parameters.""" + super().__init__(wf_params, options) + self.condition_column = wf_params.get("condition_column") + self.has_condition = False + self.condition_values = set() + + def on_header(self, header): + """Track whether reference-level validation should run.""" + self.has_condition = ( + self.wf_params.get("de_analysis") + and self.condition_column is not None + and self.condition_column in header + ) + + def add_sheet_row(self, row, lineno): + """Collect condition values for reference-level validation.""" + if not self.has_condition: + return + + condition = row.get(self.condition_column) + if not condition: + return + + self.condition_values.add(condition) + + @property + def is_valid(self): + """Validate the configured or inferred reference level.""" + if not self.has_condition: + return super().is_valid + + reference_level = self.wf_params.get("reference_level") + if reference_level is None: + if "control" in self.condition_values: + reference_level = "control" + else: + self.log_error( + ( + "Provide --reference_level when the condition column " + "does not match the default 'control'." + ) + ) + return False + + if reference_level not in self.condition_values: + self.log_error(( + f"The requested reference level '{reference_level}' is not present " + "in the condition column.")) + return False + + return super().is_valid diff --git a/bin/workflow_glue_r/R/bambu.R b/bin/workflow_glue_r/R/bambu.R index 0c6198d..0e29769 100644 --- a/bin/workflow_glue_r/R/bambu.R +++ b/bin/workflow_glue_r/R/bambu.R @@ -160,35 +160,10 @@ bambu_resolve_chunk_dirs <- function(args) { chunk_dirs } -bambu_read_sample_sheet <- function(path) { - header <- names(utils::read.csv(path, nrows = 0, check.names = FALSE)) - char_cols <- intersect(c("alias", "sample_id"), header) - col_classes <- stats::setNames(rep("character", length(char_cols)), char_cols) - utils::read.csv( - path, - check.names = FALSE, - stringsAsFactors = FALSE, - colClasses = col_classes - ) -} - bambu_resolve_inputs <- function(args, bamfile_list_ctor = Rsamtools::BamFileList) { sample_df <- NULL if (!bambu_missing(args$sample_sheet)) { - sample_df <- bambu_read_sample_sheet(args$sample_sheet) - if (!"alias" %in% names(sample_df)) { - stop("Sample sheet must contain an 'alias' column.", call. = FALSE) - } - duplicate_sample_aliases <- unique(sample_df$alias[duplicated(sample_df$alias)]) - if (length(duplicate_sample_aliases) > 0) { - stop( - sprintf( - "Sample sheet aliases must be unique; duplicated aliases: %s", - paste(duplicate_sample_aliases, collapse = ", ") - ), - call. = FALSE - ) - } + sample_df <- workflow_glue_r_read_sample_sheet(args$sample_sheet) } bam_paths <- workflow_glue_r_parse_csv_list(args$bams) @@ -1052,7 +1027,7 @@ bambu_collate_chunk_outputs <- function( }) raw_se <- bambu_combine_transcript_chunks(tx_ses) - sample_df <- bambu_read_sample_sheet(file.path(chunk_dirs[[1]], "samples.csv")) + sample_df <- workflow_glue_r_read_sample_sheet(file.path(chunk_dirs[[1]], "samples.csv")) gene_se <- gene_expression_fn(raw_se) filtered <- bambu_filter_transcripts(raw_se) diff --git a/bin/workflow_glue_r/R/common.R b/bin/workflow_glue_r/R/common.R index 77b334b..b41590d 100644 --- a/bin/workflow_glue_r/R/common.R +++ b/bin/workflow_glue_r/R/common.R @@ -128,4 +128,16 @@ workflow_glue_r_annotation_name_maps <- function(annotation_path) { annotation_meta, "transcript_id", "transcript_name", "TXNAME" ) ) +} + +workflow_glue_r_read_sample_sheet <- function(path) { + header <- names(utils::read.csv(path, nrows = 0, check.names = FALSE)) + char_cols <- intersect(c("alias", "sample_id"), header) + col_classes <- stats::setNames(rep("character", length(char_cols)), char_cols) + utils::read.csv( + path, + check.names = FALSE, + stringsAsFactors = FALSE, + colClasses = col_classes + ) } \ No newline at end of file diff --git a/bin/workflow_glue_r/R/de_analysis.R b/bin/workflow_glue_r/R/de_analysis.R index cfca013..80e5105 100644 --- a/bin/workflow_glue_r/R/de_analysis.R +++ b/bin/workflow_glue_r/R/de_analysis.R @@ -64,37 +64,6 @@ de_validate_inputs <- function(tx_se, gene_se, sample_df, argv) { label = "Design column" ) - if (!"alias" %in% names(sample_df)) { - stop("Sample sheet must contain an 'alias' column.", call. = FALSE) - } - duplicate_sample_aliases <- unique(sample_df$alias[duplicated(sample_df$alias)]) - if (length(duplicate_sample_aliases) > 0) { - stop( - sprintf( - "Sample sheet aliases must be unique; duplicated aliases: %s", - paste(duplicate_sample_aliases, collapse = ", ") - ), - call. = FALSE - ) - } - if (!(argv$condition_column %in% names(sample_df))) { - stop( - sprintf("Sample sheet must contain the '%s' column.", argv$condition_column), - call. = FALSE - ) - } - - missing_covariates <- setdiff(covariates, names(sample_df)) - if (length(missing_covariates) > 0) { - stop( - sprintf( - "Missing covariate columns: %s", - paste(missing_covariates, collapse = ", ") - ), - call. = FALSE - ) - } - if (any(duplicated(colnames(tx_se)))) { stop("Transcript RDS sample names must be unique.", call. = FALSE) } @@ -145,17 +114,7 @@ de_validate_inputs <- function(tx_se, gene_se, sample_df, argv) { reference_level <- argv$reference_level if (is.null(reference_level)) { - if ("control" %in% condition_values) { - reference_level <- "control" - } else { - stop( - "Provide --reference_level when the condition column does not contain 'control'.", - call. = FALSE - ) - } - } - if (!(reference_level %in% condition_values)) { - stop("The requested reference level is not present in the condition column.", call. = FALSE) + reference_level <- "control" } sample_df[[argv$condition_column]] <- factor(sample_df[[argv$condition_column]]) @@ -788,11 +747,7 @@ main_run_de_analysis <- function(args) { tx_se <- readRDS(args$transcript_rds) gene_se <- readRDS(args$gene_rds) - sample_df <- utils::read.csv( - args$sample_sheet, - check.names = FALSE, - stringsAsFactors = FALSE - ) + sample_df <- workflow_glue_r_read_sample_sheet(args$sample_sheet) validated <- de_validate_inputs(tx_se, gene_se, sample_df, args) sample_df <- validated$sample_df covariates <- validated$covariates diff --git a/bin/workflow_glue_r/tests/testthat/test_bambu.R b/bin/workflow_glue_r/tests/testthat/test_bambu.R index 0f654d0..d301164 100644 --- a/bin/workflow_glue_r/tests/testthat/test_bambu.R +++ b/bin/workflow_glue_r/tests/testthat/test_bambu.R @@ -135,41 +135,6 @@ testthat::test_that("unique sample aliases required", { bambu_resolve_inputs(args, bamfile_list_ctor = function(paths, yieldSize) paths), "Provide one alias per BAM in --bams" ) - - missing_alias_sheet <- tempfile(fileext = ".csv") - writeLines( - paste( - "condition", - "control", - sep = "\n" - ), - missing_alias_sheet - ) - args <- list( - bams = "sampleA.bam", - aliases = "sampleA", - sample_sheet = missing_alias_sheet - ) - testthat::expect_error( - bambu_resolve_inputs(args, bamfile_list_ctor = function(paths, yieldSize) paths), - "Sample sheet must contain an 'alias' column" - ) - - duplicate_alias_sheet <- tempfile(fileext = ".csv") - writeLines( - paste( - "alias,condition", - "sampleA,control", - "sampleA,treated", - sep = "\n" - ), - duplicate_alias_sheet - ) - args$sample_sheet <- duplicate_alias_sheet - testthat::expect_error( - bambu_resolve_inputs(args, bamfile_list_ctor = function(paths, yieldSize) paths), - "Sample sheet aliases must be unique" - ) }) # Sample sheet rows must align with BAM file order. diff --git a/bin/workflow_glue_r/tests/testthat/test_de_analysis.R b/bin/workflow_glue_r/tests/testthat/test_de_analysis.R index e78e625..a4c042d 100644 --- a/bin/workflow_glue_r/tests/testthat/test_de_analysis.R +++ b/bin/workflow_glue_r/tests/testthat/test_de_analysis.R @@ -13,53 +13,6 @@ testthat::test_that("covariates parsed and trimmed", { ) }) -# Sample sheet must have 'alias' column matching SE colnames, condition column, and all covariates. -# Aliases must be unique. Missing columns should fail fast before passing to DESeq2/DRIMSeq. -testthat::test_that("sample sheet structure validated", { - tx_se <- make_test_tx_se() - gene_se <- make_test_gene_se() - argv <- list( - transcript_rds = "transcripts.rds", - gene_rds = "genes.rds", - sample_sheet = "sample_sheet.csv", - condition_column = "condition", - covariates = "batch", - reference_level = NULL - ) - argv <- workflow_glue_r_normalise_args(argv, de_analysis_arg_spec()) - - missing_alias <- data.frame(condition = rep(c("control", "treated"), each = 3)) - testthat::expect_error( - de_validate_inputs(tx_se, gene_se, missing_alias, argv), - "Sample sheet must contain an 'alias' column" - ) - - sample_df <- data.frame( - alias = colnames(tx_se), - batch = rep(c("b1", "b2", "b1"), 2), - stringsAsFactors = FALSE - ) - testthat::expect_error( - de_validate_inputs(tx_se, gene_se, sample_df, argv), - "Sample sheet must contain the 'condition' column" - ) - - sample_df$condition <- rep(c("control", "treated"), each = 3) - argv$covariates <- "batch,site" - testthat::expect_error( - de_validate_inputs(tx_se, gene_se, sample_df, argv), - "Missing covariate columns: site" - ) - - duplicate_aliases <- sample_df - duplicate_aliases$alias[2] <- duplicate_aliases$alias[1] - argv$covariates <- "batch" - testthat::expect_error( - de_validate_inputs(tx_se, gene_se, duplicate_aliases, argv), - "Sample sheet aliases must be unique" - ) -}) - # R formulas break on spaces/hyphens in column names (e.g., ~ `time point` produces errors). # Validate condition_column and covariate names are safe (alphanumeric + underscore). testthat::test_that("formula-unsafe column names rejected", { @@ -122,16 +75,16 @@ testthat::test_that("DTU transcript output renames contrast-specific log2fold co ) }) -# Sample aliases CAN have spaces/hyphens (they're not used in formulas, just for matching). +# Sample aliases CAN have hyphens (they're not used in formulas, just for matching). # Sample sheet rows can be in different order than SE columns - should reorder automatically. testthat::test_that("non-syntactic aliases allowed, sheets reordered", { sample_names <- c( "1control", "control-2", - "control rep 3", - "4treated", + "control _rep 3", + "04", "treated-5", - "treated rep 6" + "treated-rep.6" ) tx_se <- make_test_tx_se(sample_names = sample_names) gene_se <- make_test_gene_se(sample_names = sample_names) @@ -195,48 +148,6 @@ testthat::test_that("reference level defaults to control", { ) }) -# If no "control" level exists, user MUST provide --reference_level explicitly. -# Specified reference_level must exist in the condition column values. -testthat::test_that("explicit reference level required without control", { - tx_se <- make_test_tx_se( - sample_names = c( - "baseline_rep1", "baseline_rep2", "baseline_rep3", - "treated_rep1", "treated_rep2", "treated_rep3" - ) - ) - gene_se <- make_test_gene_se( - sample_names = c( - "baseline_rep1", "baseline_rep2", "baseline_rep3", - "treated_rep1", "treated_rep2", "treated_rep3" - ) - ) - sample_df <- data.frame( - alias = colnames(tx_se), - condition = rep(c("baseline", "treated"), each = 3), - batch = rep(c("b1", "b2", "b1"), 2), - stringsAsFactors = FALSE - ) - argv <- list( - transcript_rds = "transcripts.rds", - gene_rds = "genes.rds", - sample_sheet = "sample_sheet.csv", - condition_column = "condition", - covariates = "batch", - reference_level = NULL - ) - argv <- workflow_glue_r_normalise_args(argv, de_analysis_arg_spec()) - - testthat::expect_error( - de_validate_inputs(tx_se, gene_se, sample_df, argv), - "Provide --reference_level" - ) - - argv$reference_level <- "does_not_exist" - testthat::expect_error( - de_validate_inputs(tx_se, gene_se, sample_df, argv), - "requested reference level is not present" - ) -}) # Count matrices must be numeric, non-NA, non-zero totals per sample. # Transcript and gene SE sample names must match exactly (same order, same values). diff --git a/main.nf b/main.nf index c72fbbc..91cd914 100644 --- a/main.nf +++ b/main.nf @@ -236,6 +236,24 @@ workflow { "force_alignment": params.force_alignment, ] + ingress_args, ref_genome) } + + + sample_sheet_aliases = sample_sheet == OPTIONAL_FILE ? + null : + sample_sheet + .splitCsv(header: true, quote: '"') + .collect { it.alias } + .findAll { it != null } + .toSet() + samples.subscribe { meta, xam, xai, stats -> + if (sample_sheet_aliases != null && !sample_sheet_aliases.contains(meta.alias)) { + throw new Exception( + "Sample alias '${meta.alias}' was not found in the sample_sheet alias column." + ) + } + } + + analysis_samples = samples .filter { meta, xam, xai, stats -> boolean is_excluded = false diff --git a/subworkflows/differential_expression.nf b/subworkflows/differential_expression.nf index aafeada..645fc4c 100644 --- a/subworkflows/differential_expression.nf +++ b/subworkflows/differential_expression.nf @@ -1,28 +1,6 @@ nextflow.enable.dsl = 2 -process checkExperimentDesign { - label "wf_common" - cpus 1 - memory "2 GB" - input: - path sample_sheet - output: - path "validated.ok", emit: ok - script: - String covariates_arg = params.covariates ? "--covariates '${params.covariates}'" : "" - String reference_arg = params.reference_level ? "--reference_level '${params.reference_level}'" : "" - """ - workflow-glue check_experiment_design \ - --sample_sheet "${sample_sheet}" \ - --condition_column "${params.condition_column}" \ - ${covariates_arg} \ - ${reference_arg} - touch validated.ok - """ -} - - process runDifferentialAnalysis { label "wf_transcriptomes" cpus { params.threads ?: 4 } @@ -31,7 +9,6 @@ process runDifferentialAnalysis { path transcript_rds path gene_rds path sample_sheet - path validation_token output: path "de_analysis", emit: dir script: @@ -56,8 +33,7 @@ workflow differential_expression { gene_rds sample_sheet main: - validated = checkExperimentDesign(sample_sheet) - results = runDifferentialAnalysis(transcript_rds, gene_rds, sample_sheet, validated.ok) + results = runDifferentialAnalysis(transcript_rds, gene_rds, sample_sheet) emit: dir = results.dir }