From 6d297ed5aaf29938dc24ee76cc56af0bd43ab153 Mon Sep 17 00:00:00 2001 From: Don Freed Date: Wed, 28 May 2025 15:35:01 -0700 Subject: [PATCH 1/9] Update version --- pyproject.toml | 2 +- sentieon_cli/util.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c9caca0..bc65b04 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -2,7 +2,7 @@ #https://stackoverflow.com/questions/75408641/whats-difference-between-tool-poetry-and-project-in-pyproject-toml [tool.poetry] name = "sentieon_cli" -version = "1.2.3" +version = "2.0.0" description = "entry point for sentieon command-line tools" authors = ["Don Freed ", "Brent "] readme = "README.md" diff --git a/sentieon_cli/util.py b/sentieon_cli/util.py index c310fb3..a90a781 100644 --- a/sentieon_cli/util.py +++ b/sentieon_cli/util.py @@ -16,7 +16,7 @@ from .logging import get_logger -__version__ = "1.2.3" +__version__ = "2.0.0" logger = get_logger(__name__) From 1dee9a9103637197e3c105e8d874d2865533ea1d Mon Sep 17 00:00:00 2001 From: Don Freed Date: Wed, 28 May 2025 16:43:53 -0700 Subject: [PATCH 2/9] Reorder command line arguments Hide some rarely used arguments from the command-line. --- sentieon_cli/dnascope.py | 159 +++++++++++++++--------------- sentieon_cli/dnascope_hybrid.py | 146 +++++++++++++-------------- sentieon_cli/dnascope_longread.py | 139 +++++++++++++------------- 3 files changed, 225 insertions(+), 219 deletions(-) diff --git a/sentieon_cli/dnascope.py b/sentieon_cli/dnascope.py index 8d352e6..18a0040 100644 --- a/sentieon_cli/dnascope.py +++ b/sentieon_cli/dnascope.py @@ -75,16 +75,10 @@ class DNAscopePipeline(BasePipeline): """The DNAscope pipeline""" params: Dict[str, Dict[str, Any]] = { - "reference": { - "flags": ["-r", "--reference"], + "model_bundle": { + "flags": ["-m", "--model_bundle"], + "help": "The model bundle file.", "required": True, - "help": "fasta for reference genome.", - "type": path_arg(exists=True, is_file=True), - }, - "sample_input": { - "flags": ["-i", "--sample_input"], - "nargs": "*", - "help": "sample BAM or CRAM file.", "type": path_arg(exists=True, is_file=True), }, "r1_fastq": { @@ -101,19 +95,36 @@ class DNAscopePipeline(BasePipeline): "nargs": "*", "help": "Readgroup information for the fastq files.", }, - "model_bundle": { - "flags": ["-m", "--model_bundle"], - "help": "The model bundle file.", + "reference": { + "flags": ["-r", "--reference"], "required": True, + "help": "fasta for reference genome.", "type": path_arg(exists=True, is_file=True), }, - "dbsnp": { - "flags": ["-d", "--dbsnp"], + "sample_input": { + "flags": ["-i", "--sample_input"], + "nargs": "*", + "help": "sample BAM or CRAM file.", + "type": path_arg(exists=True, is_file=True), + }, + "align": { "help": ( - "dbSNP vcf file Supplying this file will annotate variants " - "with their dbSNP refSNP ID numbers." + "Align the reads in the input uBAM/uCRAM file to the " + "reference genome. Assumes paired reads are collated in the " + "input file." ), - "type": path_arg(exists=True, is_file=True), + "action": "store_true", + }, + "assay": { + "help": "The type of assay, WGS or WES.", + "choices": ["WGS", "WES"], + "default": "WGS", + }, + "bam_format": { + "help": ( + "Use the BAM format instead of CRAM for output aligned files" + ), + "action": "store_true", }, "bed": { "flags": ["-b", "--bed"], @@ -123,9 +134,17 @@ class DNAscopePipeline(BasePipeline): ), "type": path_arg(exists=True, is_file=True), }, - "interval_padding": { - "help": "Amount to pad all intervals.", - "type": int, + "collate_align": { + "help": ( + "Collate and align the reads in the input BAM/CRAM file to " + "the reference genome. Suitable for coordinate-sorted " + "BAM/CRAM input." + ), + "action": "store_true", + }, + "consensus": { + "help": "Generate consensus reads during dedup", + "action": "store_true", }, "cores": { "flags": ["-t", "--cores"], @@ -135,16 +154,16 @@ class DNAscopePipeline(BasePipeline): ), "default": mp.cpu_count(), }, - "pcr_free": { - "help": "Use arguments for PCR-free data processing", - "action": "store_true", - }, - "gvcf": { - "flags": ["-g", "--gvcf"], + "dbsnp": { + "flags": ["-d", "--dbsnp"], "help": ( - "Generate a gVCF output file along with the VCF." - " (default generates only the VCF)" + "dbSNP vcf file Supplying this file will annotate variants " + "with their dbSNP refSNP ID numbers." ), + "type": path_arg(exists=True, is_file=True), + }, + "dry_run": { + "help": "Print the commands without running them.", "action": "store_true", }, "duplicate_marking": { @@ -152,25 +171,27 @@ class DNAscopePipeline(BasePipeline): "choices": ["markdup", "rmdup", "none"], "default": "markdup", }, - "assay": { - "help": "The type of assay, WGS or WES.", - "choices": ["WGS", "WES"], - "default": "WGS", - }, - "consensus": { - "help": "Generate consensus reads during dedup", + "gvcf": { + "flags": ["-g", "--gvcf"], + "help": ( + "Generate a gVCF output file along with the VCF." + " (default generates only the VCF)" + ), "action": "store_true", }, - "dry_run": { - "help": "Print the commands without running them.", - "action": "store_true", + "input_ref": { + "help": ( + "Used to decode the input alignment file. Required if the " + "input file is in the CRAM/uCRAM formats." + ), + "type": path_arg(exists=True, is_file=True), }, - "skip_small_variants": { - "help": "Skip small variant (SNV/indel) calling", - "action": "store_true", + "interval_padding": { + "help": "Amount to pad all intervals.", + "type": int, }, - "skip_svs": { - "help": "Skip SV calling", + "pcr_free": { + "help": "Use arguments for PCR-free data processing", "action": "store_true", }, "skip_metrics": { @@ -181,50 +202,23 @@ class DNAscopePipeline(BasePipeline): "help": "Skip multiQC report generation", "action": "store_true", }, - "align": { - "help": ( - "Align the reads in the input uBAM/uCRAM file to the " - "reference genome. Assumes paired reads are collated in the " - "input file." - ), - "action": "store_true", - }, - "collate_align": { - "help": ( - "Collate and align the reads in the input BAM/CRAM file to " - "the reference genome. Suitable for coordinate-sorted " - "BAM/CRAM input." - ), + "skip_small_variants": { + "help": "Skip small variant (SNV/indel) calling", "action": "store_true", }, - "input_ref": { - "help": ( - "Used to decode the input alignment file. Required if the " - "input file is in the CRAM/uCRAM formats." - ), - "type": path_arg(exists=True, is_file=True), - }, - "bam_format": { - "help": ( - "Use the BAM format instead of CRAM for output aligned files" - ), + "skip_svs": { + "help": "Skip SV calling", "action": "store_true", }, "bwa_args": { - "help": "Extra arguments for sentieon bwa", + #"help": "Extra arguments for sentieon bwa", + "help": argparse.SUPPRESS, "default": "", }, "bwa_k": { - "help": "The '-K' argument in bwa", - "default": 100000000, - }, - "util_sort_args": { - "help": "Extra arguments for sentieon util sort", - "default": "--cram_write_options version=3.0,compressor=rans", - }, - "skip_version_check": { + #"help": "The '-K' argument in bwa", "help": argparse.SUPPRESS, - "action": "store_true", + "default": 100000000, }, "bwt_max_mem": { # Manually set `bwt_max_mem` @@ -239,6 +233,15 @@ class DNAscopePipeline(BasePipeline): "help": argparse.SUPPRESS, "action": "store_true", }, + "skip_version_check": { + "help": argparse.SUPPRESS, + "action": "store_true", + }, + "util_sort_args": { + #"help": "Extra arguments for sentieon util sort", + "help": argparse.SUPPRESS, + "default": "--cram_write_options version=3.0,compressor=rans", + }, } positionals: Dict[str, Dict[str, Any]] = { diff --git a/sentieon_cli/dnascope_hybrid.py b/sentieon_cli/dnascope_hybrid.py index f0db0f5..103a701 100644 --- a/sentieon_cli/dnascope_hybrid.py +++ b/sentieon_cli/dnascope_hybrid.py @@ -52,12 +52,29 @@ class DNAscopeHybridPipeline(DNAscopePipeline, DNAscopeLRPipeline): """The DNAscope Hybrid pipeline""" params: Dict[str, Dict[str, Any]] = { + "lr_aln": { + "nargs": "*", + "help": "Long-read BAM or CRAM files.", + "type": path_arg(exists=True, is_file=True), + "required": True, + }, + "model_bundle": { + "flags": ["-m", "--model_bundle"], + "help": "The model bundle file.", + "required": True, + "type": path_arg(exists=True, is_file=True), + }, "reference": { "flags": ["-r", "--reference"], "required": True, "help": "fasta for reference genome.", "type": path_arg(exists=True, is_file=True), }, + "sr_aln": { + "nargs": "*", + "help": "Short-read BAM or CRAM files", + "type": path_arg(exists=True, is_file=True), + }, "sr_r1_fastq": { "nargs": "*", "help": "Short-read R1 fastq files", @@ -72,35 +89,11 @@ class DNAscopeHybridPipeline(DNAscopePipeline, DNAscopeLRPipeline): "nargs": "*", "help": "Readgroup information for the short-read fastq files", }, - "sr_aln": { - "nargs": "*", - "help": "Short-read BAM or CRAM files", - "type": path_arg(exists=True, is_file=True), - }, - "sr_duplicate_marking": { - "help": "Options for duplicate marking.", - "choices": ["markdup", "rmdup", "none"], - "default": "markdup", - }, - "lr_aln": { - "nargs": "*", - "help": "Long-read BAM or CRAM files.", - "type": path_arg(exists=True, is_file=True), - "required": True, - }, - "model_bundle": { - "flags": ["-m", "--model_bundle"], - "help": "The model bundle file.", - "required": True, - "type": path_arg(exists=True, is_file=True), - }, - "dbsnp": { - "flags": ["-d", "--dbsnp"], + "bam_format": { "help": ( - "dbSNP vcf file Supplying this file will annotate variants " - "with their dbSNP refSNP ID numbers." + "Use the BAM format instead of CRAM for output aligned files" ), - "type": path_arg(exists=True, is_file=True), + "action": "store_true", }, "bed": { "flags": ["-b", "--bed"], @@ -118,36 +111,24 @@ class DNAscopeHybridPipeline(DNAscopePipeline, DNAscopeLRPipeline): ), "default": mp.cpu_count(), }, - "gvcf": { - "flags": ["-g", "--gvcf"], + "dbsnp": { + "flags": ["-d", "--dbsnp"], "help": ( - "Generate a gVCF output file along with the VCF." - " (default generates only the VCF)" + "dbSNP vcf file Supplying this file will annotate variants " + "with their dbSNP refSNP ID numbers." ), - "action": "store_true", + "type": path_arg(exists=True, is_file=True), }, "dry_run": { "help": "Print the commands without running them.", "action": "store_true", }, - "skip_svs": { - "help": "Skip SV calling", - "action": "store_true", - }, - "skip_metrics": { - "help": "Skip all metrics collection and multiQC", - "action": "store_true", - }, - "skip_mosdepth": { - "help": "Skip QC with mosdepth.", - "action": "store_true", - }, - "skip_multiqc": { - "help": "Skip multiQC report generation.", - "action": "store_true", - }, - "skip_cnv": { - "help": "Skip CNV calling.", + "gvcf": { + "flags": ["-g", "--gvcf"], + "help": ( + "Generate a gVCF output file along with the VCF." + " (default generates only the VCF)" + ), "action": "store_true", }, "lr_align_input": { @@ -164,23 +145,36 @@ class DNAscopeHybridPipeline(DNAscopePipeline, DNAscopeLRPipeline): ), "type": path_arg(exists=True, is_file=True), }, - "bam_format": { - "help": ( - "Use the BAM format instead of CRAM for output aligned files" - ), - "action": "store_true", - }, "rgsm": { "help": ( "Overwrite the SM tag of the input readgroups for " "compatibility" ) }, - "lr_fastq_taglist": { - # help="A comma-separated list of tags to retain. Defaults to " - # "'%(default)s' and the 'RG' tag is required", - "help": argparse.SUPPRESS, - "default": "*", + "skip_cnv": { + "help": "Skip CNV calling.", + "action": "store_true", + }, + "skip_metrics": { + "help": "Skip all metrics collection and multiQC", + "action": "store_true", + }, + "skip_mosdepth": { + "help": "Skip QC with mosdepth.", + "action": "store_true", + }, + "skip_multiqc": { + "help": "Skip multiQC report generation.", + "action": "store_true", + }, + "skip_svs": { + "help": "Skip SV calling", + "action": "store_true", + }, + "sr_duplicate_marking": { + "help": "Options for duplicate marking.", + "choices": ["markdup", "rmdup", "none"], + "default": "markdup", }, "bwa_args": { # help="Extra arguments for sentieon bwa", @@ -192,17 +186,25 @@ class DNAscopeHybridPipeline(DNAscopePipeline, DNAscopeLRPipeline): "help": argparse.SUPPRESS, "default": 100000000, }, + "bwt_max_mem": { + # Manually set `bwt_max_mem` + "help": argparse.SUPPRESS, + }, + "lr_fastq_taglist": { + # help="A comma-separated list of tags to retain. Defaults to " + # "'%(default)s' and the 'RG' tag is required", + "help": argparse.SUPPRESS, + "default": "*", + }, + "lr_read_filter": { + "help": argparse.SUPPRESS, + }, "minimap2_args": { # help="Extra arguments for sentieon minimap2", "help": argparse.SUPPRESS, "default": "-Y", }, - "util_sort_args": { - # help="Extra arguments for sentieon util sort", - "help": argparse.SUPPRESS, - "default": "--cram_write_options version=3.0,compressor=rans", - }, - "skip_version_check": { + "no_split_alignment": { "help": argparse.SUPPRESS, "action": "store_true", }, @@ -210,19 +212,17 @@ class DNAscopeHybridPipeline(DNAscopePipeline, DNAscopeLRPipeline): "help": argparse.SUPPRESS, "action": "store_true", }, - "bwt_max_mem": { - # Manually set `bwt_max_mem` - "help": argparse.SUPPRESS, - }, - "no_split_alignment": { + "skip_version_check": { "help": argparse.SUPPRESS, "action": "store_true", }, "sr_read_filter": { "help": argparse.SUPPRESS, }, - "lr_read_filter": { + "util_sort_args": { + # help="Extra arguments for sentieon util sort", "help": argparse.SUPPRESS, + "default": "--cram_write_options version=3.0,compressor=rans", }, } diff --git a/sentieon_cli/dnascope_longread.py b/sentieon_cli/dnascope_longread.py index 8d45c41..1f89364 100644 --- a/sentieon_cli/dnascope_longread.py +++ b/sentieon_cli/dnascope_longread.py @@ -74,6 +74,21 @@ class DNAscopeLRPipeline(BasePipeline): """The DNAscope LongRead pipeline""" params: Dict[str, Dict[str, Any]] = { + "fastq": { + "nargs": "*", + "help": "Sample fastq files.", + "type": path_arg(exists=True, is_file=True), + }, + "model_bundle": { + "flags": ["-m", "--model_bundle"], + "help": "The model bundle file.", + "required": True, + "type": path_arg(exists=True, is_file=True), + }, + "readgroups": { + "nargs": "*", + "help": "Readgroup information for the fastq files.", + }, "reference": { "flags": ["-r", "--reference"], "required": True, @@ -86,28 +101,17 @@ class DNAscopeLRPipeline(BasePipeline): "help": "sample BAM or CRAM file.", "type": path_arg(exists=True, is_file=True), }, - "fastq": { - "nargs": "*", - "help": "Sample fastq files.", - "type": path_arg(exists=True, is_file=True), - }, - "readgroups": { - "nargs": "*", - "help": "Readgroup information for the fastq files.", - }, - "model_bundle": { - "flags": ["-m", "--model_bundle"], - "help": "The model bundle file.", - "required": True, - "type": path_arg(exists=True, is_file=True), + "align": { + "help": ( + "Align the input BAM/CRAM/uBAM file to the reference genome." + ), + "action": "store_true", }, - "dbsnp": { - "flags": ["-d", "--dbsnp"], + "bam_format": { "help": ( - "dbSNP vcf file Supplying this file will annotate variants " - "with their dbSNP refSNP ID numbers." + "Use the BAM format instead of CRAM for output aligned files." ), - "type": path_arg(exists=True, is_file=True), + "action": "store_true", }, "bed": { "flags": ["-b", "--bed"], @@ -117,13 +121,6 @@ class DNAscopeLRPipeline(BasePipeline): ), "type": path_arg(exists=True, is_file=True), }, - "haploid_bed": { - "help": ( - "A BED file of haploid regions. Supplying this file will " - "perform haploid variant calling across these regions." - ), - "type": path_arg(exists=True, is_file=True), - }, "cnv_excluded_regions": { "help": "Regions to exclude from CNV calling.", "type": path_arg(exists=True, is_file=True), @@ -136,6 +133,18 @@ class DNAscopeLRPipeline(BasePipeline): ), "default": mp.cpu_count(), }, + "dbsnp": { + "flags": ["-d", "--dbsnp"], + "help": ( + "dbSNP vcf file Supplying this file will annotate variants " + "with their dbSNP refSNP ID numbers." + ), + "type": path_arg(exists=True, is_file=True), + }, + "dry_run": { + "help": "Print the commands without running them.", + "action": "store_true", + }, "gvcf": { "flags": ["-g", "--gvcf"], "help": ( @@ -144,74 +153,63 @@ class DNAscopeLRPipeline(BasePipeline): ), "action": "store_true", }, - "tech": { - "help": "Sequencing technology used to generate the reads.", - "choices": ["HiFi", "ONT"], - "default": "HiFi", - }, - "dry_run": { - "help": "Print the commands without running them.", - "action": "store_true", + "haploid_bed": { + "help": ( + "A BED file of haploid regions. Supplying this file will " + "perform haploid variant calling across these regions." + ), + "type": path_arg(exists=True, is_file=True), }, - "skip_small_variants": { - "help": "Skip small variant (SNV/indel) calling.", - "action": "store_true", + "input_ref": { + "help": ( + "Used to decode the input alignment file. Required if the " + "input file is in the CRAM/uCRAM formats." + ), + "type": path_arg(exists=True, is_file=True), }, - "skip_svs": { - "help": "Skip SV calling.", + "skip_cnv": { + "help": "Skip CNV calling.", "action": "store_true", }, "skip_mosdepth": { "help": "Skip QC with mosdepth.", "action": "store_true", }, - "skip_cnv": { - "help": "Skip CNV calling.", + "skip_small_variants": { + "help": "Skip small variant (SNV/indel) calling.", "action": "store_true", }, - "align": { - "help": ( - "Align the input BAM/CRAM/uBAM file to the reference genome." - ), + "skip_svs": { + "help": "Skip SV calling.", "action": "store_true", }, - "input_ref": { - "help": ( - "Used to decode the input alignment file. Required if the " - "input file is in the CRAM/uCRAM formats." - ), - "type": path_arg(exists=True, is_file=True), + "tech": { + "help": "Sequencing technology used to generate the reads.", + "choices": ["HiFi", "ONT"], + "default": "HiFi", }, "fastq_taglist": { - "help": ( - "A comma-separated list of tags to retain. Defaults to " - "'%(default)s' and the 'RG' tag is required." - ), + #"help": ( + # "A comma-separated list of tags to retain. Defaults to " + # "'%(default)s' and the 'RG' tag is required." + #), + "help": argparse.SUPPRESS, "default": "*", }, - "bam_format": { - "help": ( - "Use the BAM format instead of CRAM for output aligned files." - ), - "action": "store_true", - }, "minimap2_args": { - "help": "Extra arguments for sentieon minimap2.", + #"help": "Extra arguments for sentieon minimap2.", + "help": argparse.SUPPRESS, "default": "-Y", }, - "util_sort_args": { - "help": "Extra arguments for sentieon util sort.", - "default": "--cram_write_options version=3.0,compressor=rans", - }, "repeat_model": { "help": argparse.SUPPRESS, "type": path_arg(exists=True, is_file=True), }, - "skip_version_check": { + "retain_tmpdir": { "help": argparse.SUPPRESS, "action": "store_true", }, - "retain_tmpdir": { + "skip_version_check": { "help": argparse.SUPPRESS, "action": "store_true", }, @@ -219,6 +217,11 @@ class DNAscopeLRPipeline(BasePipeline): "help": argparse.SUPPRESS, "action": "store_true", }, + "util_sort_args": { + #"help": "Extra arguments for sentieon util sort.", + "help": argparse.SUPPRESS, + "default": "--cram_write_options version=3.0,compressor=rans", + }, } positionals: Dict[str, Dict[str, Any]] = { From 9c3ec7f6ea136eeffbfb27f16e5a4a9900b10a15 Mon Sep 17 00:00:00 2001 From: Don Freed Date: Thu, 5 Jun 2025 22:09:03 -0700 Subject: [PATCH 3/9] Workaround if lscpu is unavailable --- sentieon_cli/util.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/sentieon_cli/util.py b/sentieon_cli/util.py index a90a781..ba7e220 100644 --- a/sentieon_cli/util.py +++ b/sentieon_cli/util.py @@ -3,6 +3,7 @@ """ import argparse +import multiprocessing as mp import os import pathlib import re @@ -139,14 +140,17 @@ def total_memory() -> int: def find_numa_nodes() -> List[str]: """Find NUMA nodes on the system, if available""" - res = sp.run("lscpu", capture_output=True, text=True) numa_nodes = [] - for line in res.stdout.split("\n"): - m = NUMA_NODE_PAT.match(line) - if m: - cpus = m.groupdict()["cpus"] - numa_nodes.append(cpus) - logger.debug("Identified NUMA nodes: %s", numa_nodes) + try: + res = sp.run("lscpu", capture_output=True, text=True) + for line in res.stdout.split("\n"): + m = NUMA_NODE_PAT.match(line) + if m: + cpus = m.groupdict()["cpus"] + numa_nodes.append(cpus) + logger.debug("Identified NUMA nodes: %s", numa_nodes) + except FileNotFoundError: + numa_nodes = ["0-" + str(mp.cpu_count() - 1)] return numa_nodes @@ -167,7 +171,7 @@ def split_numa_nodes(numa_nodes: List[str]) -> List[str]: return new_numa_nodes -def spit_alignment(cores: int) -> List[str]: +def split_alignment(cores: int) -> List[str]: """split large alignment tasks into smaller parts on large machines""" numa_nodes = find_numa_nodes() while len(numa_nodes) > 0 and cores / len(numa_nodes) >= 48: From f02259dbfff4d8ddf9998224875e9f6ad2c5bbd0 Mon Sep 17 00:00:00 2001 From: Don Freed Date: Thu, 5 Jun 2025 22:09:41 -0700 Subject: [PATCH 4/9] Fix a typo --- sentieon_cli/dnascope.py | 4 ++-- sentieon_cli/dnascope_hybrid.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sentieon_cli/dnascope.py b/sentieon_cli/dnascope.py index 18a0040..3a48cc3 100644 --- a/sentieon_cli/dnascope.py +++ b/sentieon_cli/dnascope.py @@ -44,7 +44,7 @@ check_version, library_preloaded, path_arg, - spit_alignment, + split_alignment, total_memory, ) @@ -346,7 +346,7 @@ def configure_alignment(self) -> None: self.numa_nodes: List[str] = [] n_alignment_jobs = 1 if not self.no_split_alignment: - self.numa_nodes = spit_alignment(self.cores) + self.numa_nodes = split_alignment(self.cores) n_alignment_jobs = max(1, len(self.numa_nodes)) total_input_size = 0 diff --git a/sentieon_cli/dnascope_hybrid.py b/sentieon_cli/dnascope_hybrid.py index 103a701..e44e840 100644 --- a/sentieon_cli/dnascope_hybrid.py +++ b/sentieon_cli/dnascope_hybrid.py @@ -36,7 +36,7 @@ library_preloaded, parse_rg_line, path_arg, - spit_alignment, + split_alignment, ) @@ -394,7 +394,7 @@ def configure(self) -> None: self.numa_nodes: List[str] = [] n_alignment_jobs = 1 if not self.no_split_alignment: - self.numa_nodes = spit_alignment(self.cores) + self.numa_nodes = split_alignment(self.cores) n_alignment_jobs = max(1, len(self.numa_nodes)) self.set_bwt_max_mem(0, n_alignment_jobs) From 56c50edd847ebd08c42dcef3b1710b7e29c2b9d3 Mon Sep 17 00:00:00 2001 From: Don Freed Date: Thu, 5 Jun 2025 22:33:16 -0700 Subject: [PATCH 5/9] Formatting --- sentieon_cli/command_strings.py | 2 +- sentieon_cli/dnascope.py | 6 +++--- sentieon_cli/dnascope_longread.py | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sentieon_cli/command_strings.py b/sentieon_cli/command_strings.py index bd5b838..92ca3a6 100644 --- a/sentieon_cli/command_strings.py +++ b/sentieon_cli/command_strings.py @@ -451,7 +451,7 @@ def bcftools_subset( ] ) return ( - f'if [ -s "{regions_bed}" ]; then ' + f'if [ -s "{regions_bed}" ]; then ' # noqa + bcftools_subset_cmd + " | " + vcfconvert_cmd diff --git a/sentieon_cli/dnascope.py b/sentieon_cli/dnascope.py index 3a48cc3..5a62e27 100644 --- a/sentieon_cli/dnascope.py +++ b/sentieon_cli/dnascope.py @@ -211,12 +211,12 @@ class DNAscopePipeline(BasePipeline): "action": "store_true", }, "bwa_args": { - #"help": "Extra arguments for sentieon bwa", + # "help": "Extra arguments for sentieon bwa", "help": argparse.SUPPRESS, "default": "", }, "bwa_k": { - #"help": "The '-K' argument in bwa", + # "help": "The '-K' argument in bwa", "help": argparse.SUPPRESS, "default": 100000000, }, @@ -238,7 +238,7 @@ class DNAscopePipeline(BasePipeline): "action": "store_true", }, "util_sort_args": { - #"help": "Extra arguments for sentieon util sort", + # "help": "Extra arguments for sentieon util sort", "help": argparse.SUPPRESS, "default": "--cram_write_options version=3.0,compressor=rans", }, diff --git a/sentieon_cli/dnascope_longread.py b/sentieon_cli/dnascope_longread.py index 1f89364..23b7dc4 100644 --- a/sentieon_cli/dnascope_longread.py +++ b/sentieon_cli/dnascope_longread.py @@ -189,15 +189,15 @@ class DNAscopeLRPipeline(BasePipeline): "default": "HiFi", }, "fastq_taglist": { - #"help": ( + # "help": ( # "A comma-separated list of tags to retain. Defaults to " # "'%(default)s' and the 'RG' tag is required." - #), + # ), "help": argparse.SUPPRESS, "default": "*", }, "minimap2_args": { - #"help": "Extra arguments for sentieon minimap2.", + # "help": "Extra arguments for sentieon minimap2.", "help": argparse.SUPPRESS, "default": "-Y", }, @@ -218,7 +218,7 @@ class DNAscopeLRPipeline(BasePipeline): "action": "store_true", }, "util_sort_args": { - #"help": "Extra arguments for sentieon util sort.", + # "help": "Extra arguments for sentieon util sort.", "help": argparse.SUPPRESS, "default": "--cram_write_options version=3.0,compressor=rans", }, From 2aa2f516e6c889151f2799e04bd16e6ce4eacf5b Mon Sep 17 00:00:00 2001 From: Don Freed Date: Thu, 5 Jun 2025 22:35:18 -0700 Subject: [PATCH 6/9] Add unit and integration tests --- .github/workflows/ci.yml | 4 +- tests/integration/test_pipeline_execution.py | 327 ++++++++++++++++ tests/unit/test_dag_construction.py | 366 ++++++++++++++++++ tests/unit/test_pipeline_validation.py | 376 +++++++++++++++++++ tests/utils/test_helpers.py | 367 ++++++++++++++++++ 5 files changed, 1438 insertions(+), 2 deletions(-) create mode 100644 tests/integration/test_pipeline_execution.py create mode 100644 tests/unit/test_dag_construction.py create mode 100644 tests/unit/test_pipeline_validation.py create mode 100644 tests/utils/test_helpers.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 74e96f4..549dc82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,9 +45,9 @@ jobs: - name: Run black formatter run: poetry run black --check --exclude '__pycache__|scripts|sentieon_cli/archive.py' --line-length 79 sentieon_cli - name: Run mypy - run: poetry run mypy . + run: poetry run mypy sentieon_cli - name: Run flake8 - run: poetry run flake8 . --extend-ignore E231,E221, --exclude .github/scripts/license_message.py,sentieon_cli/scripts/gvcf_combine.py,sentieon_cli/scripts/vcf_mod.py,sentieon_cli/scripts/hybrid_anno.py,sentieon_cli/scripts/hybrid_select.py,sentieon_cli/archive.py # false+ from python 3.12 + run: poetry run flake8 . --extend-ignore E231,E221, --exclude .github/scripts/license_message.py,sentieon_cli/scripts/gvcf_combine.py,sentieon_cli/scripts/vcf_mod.py,sentieon_cli/scripts/hybrid_anno.py,sentieon_cli/scripts/hybrid_select.py,sentieon_cli/archive.py,tests # false+ from python 3.12 - name: Run the automated tests run: poetry run pytest -v - name: Run doct tests diff --git a/tests/integration/test_pipeline_execution.py b/tests/integration/test_pipeline_execution.py new file mode 100644 index 0000000..f36652c --- /dev/null +++ b/tests/integration/test_pipeline_execution.py @@ -0,0 +1,327 @@ +""" +Integration tests for pipeline execution +""" + +import pytest +import sys +import os +from unittest.mock import patch + +# Add the parent directory to the path +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))) + +from tests.utils.test_helpers import ( + CommandValidator, + DAGAnalyzer, + setup_basic_test_environment, + teardown_test_environment +) + + +class TestPipelineDryRunExecution: + """Test pipeline execution in dry-run mode""" + + def setup_method(self): + """Setup test environment""" + self.helper, self.patches = setup_basic_test_environment() + + def teardown_method(self): + """Cleanup test environment""" + teardown_test_environment(self.helper, self.patches) + + def test_dnascope_dry_run_execution(self): + """Test DNAscope pipeline dry-run execution""" + pipeline = self.helper.create_dnascope_pipeline(use_bam=True) + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Extract all commands that would be executed + all_jobs = list(dag.waiting_jobs.keys()) + list(dag.ready_jobs.keys()) + commands = [job.shell for job in all_jobs] + + # Validate commands + sentieon_commands = [cmd for cmd in commands if cmd.startswith("sentieon")] + assert len(sentieon_commands) > 0, "Should have sentieon commands" + + for cmd in sentieon_commands: + assert CommandValidator.validate_sentieon_command(cmd), f"Invalid command: {cmd}" + + def test_longread_dry_run_execution(self): + """Test long-read pipeline dry-run execution""" + pipeline = self.helper.create_longread_pipeline() + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Extract all commands + all_jobs = list(dag.waiting_jobs.keys()) + list(dag.ready_jobs.keys()) + commands = [job.shell for job in all_jobs] + + # Should have complex phased calling commands + variant_commands = [cmd for cmd in commands if "DNAscope" in cmd or "VariantPhaser" in cmd] + assert len(variant_commands) > 1, "Should have multiple variant calling steps" + + def test_hybrid_dry_run_execution(self): + """Test hybrid pipeline dry-run execution""" + pipeline = self.helper.create_hybrid_pipeline() + + # Mock the archive loading and readgroup functions + with patch('sentieon_cli.dnascope_hybrid.ar_load') as mock_ar_load, \ + patch('sentieon_cli.command_strings.get_rg_lines') as mock_get_rg: + + # Mock bundle info + import json + bundle_info = { + "longReadPlatform": "HiFi", + "shortReadPlatform": "Illumina", + "minScriptVersion": "1.0.0", + "pipeline": "DNAscope Hybrid" + } + mock_ar_load.side_effect = [ + json.dumps(bundle_info).encode(), # First call for bundle_info.json + ["longreadsv.model", "cnv.model", "bwa.model"] # Second call for bundle members + ] + + # Mock readgroup lines + mock_get_rg.return_value = ["@RG\tID:test\tSM:sample"] + + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Should have hybrid-specific jobs + job_names = [job.name for job in list(dag.waiting_jobs.keys()) + list(dag.ready_jobs.keys())] + + # Look for hybrid-specific stages + hybrid_stages = [name for name in job_names if "stage" in name.lower()] + assert len(hybrid_stages) > 0, "Should have hybrid staging jobs" + + +class TestPipelineCommandGeneration: + """Test command generation for different pipeline configurations""" + + def setup_method(self): + """Setup test environment""" + self.helper, self.patches = setup_basic_test_environment() + + def teardown_method(self): + """Cleanup test environment""" + teardown_test_environment(self.helper, self.patches) + + def test_dnascope_command_parameters(self): + """Test DNAscope command parameter generation""" + pipeline = self.helper.create_dnascope_pipeline( + use_bam=True, + dbsnp=self.helper.fs.create_file("dbsnp.vcf.gz"), + pcr_free=True, + gvcf=True + ) + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Find variant calling job + variant_job = DAGAnalyzer.get_job_by_name(dag, "variant-calling") + assert variant_job is not None, "Should have variant calling job" + + # Check that command includes expected parameters + command = variant_job.shell + assert "--algo DNAscope" in command + assert "dbsnp.vcf.gz" in command, "Should include dbSNP file" + assert "emit_mode" in command, "Should have emit mode for gVCF" + + def test_resource_allocation_in_commands(self): + """Test that resource allocation is reflected in commands""" + pipeline = self.helper.create_dnascope_pipeline( + use_bam=True, + cores=8 + ) + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Check thread allocation + thread_counts = DAGAnalyzer.count_jobs_by_thread_usage(dag) + + # Should have jobs with different thread counts + assert len(thread_counts) > 1, "Should have jobs with different thread requirements" + + # High-CPU jobs should use multiple threads + high_cpu_jobs = [job for job in list(dag.waiting_jobs.keys()) + list(dag.ready_jobs.keys()) + if job.threads >= 4] + assert len(high_cpu_jobs) > 0, "Should have high-CPU jobs" + + def test_conditional_algorithm_inclusion(self): + """Test conditional inclusion of algorithms based on parameters""" + # Test with metrics enabled + pipeline1 = self.helper.create_dnascope_pipeline(use_bam=True, skip_metrics=False) + pipeline1.validate() + pipeline1.configure() + dag1 = pipeline1.build_dag() + + # Test with metrics disabled + pipeline2 = self.helper.create_dnascope_pipeline(use_bam=True, skip_metrics=True) + pipeline2.validate() + pipeline2.configure() + dag2 = pipeline2.build_dag() + + # Extract commands + commands1 = [job.shell for job in list(dag1.waiting_jobs.keys()) + list(dag1.ready_jobs.keys())] + commands2 = [job.shell for job in list(dag2.waiting_jobs.keys()) + list(dag2.ready_jobs.keys())] + + # Metrics pipeline should have more algorithms + metrics_algos1 = [cmd for cmd in commands1 if "--algo" in cmd and any( + algo in cmd for algo in ["InsertSizeMetricAlgo", "MeanQualityByCycle", "GCBias"] + )] + metrics_algos2 = [cmd for cmd in commands2 if "--algo" in cmd and any( + algo in cmd for algo in ["InsertSizeMetricAlgo", "MeanQualityByCycle", "GCBias"] + )] + + assert len(metrics_algos1) > len(metrics_algos2), "Metrics enabled should have more metric algorithms" + + +class TestPipelineErrorHandling: + """Test pipeline error handling and validation""" + + def setup_method(self): + """Setup test environment""" + self.helper, self.patches = setup_basic_test_environment() + + def teardown_method(self): + """Cleanup test environment""" + teardown_test_environment(self.helper, self.patches) + + def test_missing_required_files(self): + """Test handling of missing required files""" + pipeline = self.helper.create_dnascope_pipeline(use_bam=True) + + # Remove a required file + pipeline.reference.unlink() + + # Should handle missing file gracefully during validation + # (In practice, this might be caught by argparse or during execution) + try: + pipeline.validate() + pipeline.configure() + dag = pipeline.build_dag() + + # Commands should still be generated, but execution would fail + all_jobs = list(dag.waiting_jobs.keys()) + list(dag.ready_jobs.keys()) + assert len(all_jobs) > 0 + except Exception: + # Some pipelines might catch this during validation + pass + + def test_invalid_parameter_combinations(self): + """Test handling of invalid parameter combinations""" + pipeline = self.helper.create_longread_pipeline() + + # Set invalid combination: haploid bed without diploid bed + pipeline.haploid_bed = self.helper.fs.create_bed_file("haploid.bed") + pipeline.bed = None + pipeline.skip_small_variants = False + + with pytest.raises(SystemExit): + pipeline.validate() + + @patch('sentieon_cli.util.check_version') + def test_version_check_failure(self, mock_check_version): + """Test behavior when version checks fail""" + mock_check_version.return_value = False + + pipeline = self.helper.create_dnascope_pipeline(use_bam=True) + pipeline.skip_version_check = False # Enable version checking + + with pytest.raises(SystemExit): + pipeline.validate() + pipeline.configure() + pipeline.build_dag() + + +class TestDAGDependencyValidation: + """Test DAG dependency validation""" + + def setup_method(self): + """Setup test environment""" + self.helper, self.patches = setup_basic_test_environment() + + def teardown_method(self): + """Cleanup test environment""" + teardown_test_environment(self.helper, self.patches) + + def test_alignment_to_dedup_dependency(self): + """Test that deduplication depends on alignment""" + pipeline = self.helper.create_dnascope_pipeline() # Uses FASTQ (requires alignment) + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Dedup should depend on LC and LC on alignment + alignment_jobs = DAGAnalyzer.get_jobs_by_type(dag, "align") + lc_jobs = DAGAnalyzer.get_jobs_by_type(dag, "locuscollector") + dedup_jobs = DAGAnalyzer.get_jobs_by_type(dag, "dedup") + + lc_alignment_dep = False + dedup_lc_dep = False + for lc_job in lc_jobs: + if lc_job in dag.waiting_jobs: + deps = dag.waiting_jobs[lc_job] + if any(align_job in deps for align_job in alignment_jobs): + lc_alignment_dep = True + break + for dedup_job in dedup_jobs: + if dedup_job in dag.waiting_jobs: + deps = dag.waiting_jobs[dedup_job] + if any(lc_job in deps for lc_job in lc_jobs): + dedup_lc_dep = True + + assert lc_alignment_dep, "LocusCollector should depend on alignment" + assert dedup_lc_dep, "Dedup should depend on LocusCollector" + + def test_variant_calling_dependencies(self): + """Test that variant calling has proper dependencies""" + pipeline = self.helper.create_dnascope_pipeline(use_bam=True) + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Variant calling should depend on either dedup or input processing + variant_jobs = DAGAnalyzer.get_jobs_by_type(dag, "variant") + + assert len(variant_jobs) > 0, "Should have variant calling jobs" + + for variant_job in variant_jobs: + if variant_job in dag.waiting_jobs: + # Should have some dependencies + assert len(dag.waiting_jobs[variant_job]) > 0, "Variant calling should have dependencies" + + def test_cleanup_jobs_at_end(self): + """Test that cleanup jobs are scheduled after main processing""" + pipeline = self.helper.create_dnascope_pipeline(use_bam=True) + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Find cleanup jobs (typically have fail_ok=True) + cleanup_jobs = [job for job in list(dag.waiting_jobs.keys()) + list(dag.ready_jobs.keys()) + if job.fail_ok] + + if cleanup_jobs: + # Cleanup jobs should depend on main processing jobs + for cleanup_job in cleanup_jobs: + if cleanup_job in dag.waiting_jobs: + deps = dag.waiting_jobs[cleanup_job] + assert len(deps) > 0, "Cleanup jobs should have dependencies" + + +if __name__ == "__main__": + pytest.main([__file__]) diff --git a/tests/unit/test_dag_construction.py b/tests/unit/test_dag_construction.py new file mode 100644 index 0000000..ce3c000 --- /dev/null +++ b/tests/unit/test_dag_construction.py @@ -0,0 +1,366 @@ +""" +Unit tests for DAG construction logic +""" + +import pathlib +import pytest +import tempfile +import sys +import os +from unittest.mock import patch + +# Add the parent directory to the path so we can import sentieon_cli +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))) + +from sentieon_cli.dnascope import DNAscopePipeline +from sentieon_cli.dnascope_longread import DNAscopeLRPipeline +from sentieon_cli.dag import DAG +from sentieon_cli.job import Job + + +class TestDAGConstruction: + """Test DAG construction for pipelines""" + + def setup_method(self): + """Setup test fixtures""" + self.temp_dir = tempfile.mkdtemp() + + # Create mock files + self.mock_vcf = pathlib.Path(self.temp_dir) / "output.vcf.gz" + self.mock_ref = pathlib.Path(self.temp_dir) / "reference.fa" + self.mock_bam = pathlib.Path(self.temp_dir) / "sample.bam" + self.mock_bundle = pathlib.Path(self.temp_dir) / "model.bundle" + + # Create empty files + for file_path in [self.mock_ref, self.mock_bam, self.mock_bundle]: + file_path.touch() + + # Create reference index + (pathlib.Path(str(self.mock_ref) + ".fai")).touch() + + def create_basic_dnascope_pipeline(self): + """Create a basic DNAscope pipeline for testing""" + pipeline = DNAscopePipeline() + + # Setup logging first + from tests.utils.test_helpers import create_mock_args + args = create_mock_args() + pipeline.setup_logging(args) + + pipeline.output_vcf = self.mock_vcf + pipeline.reference = self.mock_ref + pipeline.model_bundle = self.mock_bundle + pipeline.sample_input = [self.mock_bam] + pipeline.sr_r1_fastq = [] + pipeline.sr_r2_fastq = [] + pipeline.sr_readgroups = [] + pipeline.sr_duplicate_marking = "markdup" + pipeline.cores = 2 + pipeline.dry_run = True # Important: use dry run to avoid actual execution + pipeline.skip_version_check = True + pipeline.tmp_dir = pathlib.Path(self.temp_dir) + + return pipeline + + @patch('sentieon_cli.util.library_preloaded') + def test_basic_dag_structure(self, mock_lib_preloaded): + """Test basic DAG structure is created correctly""" + mock_lib_preloaded.return_value = True + + pipeline = self.create_basic_dnascope_pipeline() + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Verify DAG is created + assert isinstance(dag, DAG) + + # Should have jobs for dedup, variant calling, etc. + total_jobs = (len(dag.waiting_jobs) + + len(dag.ready_jobs) + + len(dag.finished_jobs)) + assert total_jobs > 0 + + @patch('sentieon_cli.util.library_preloaded') + def test_dag_job_dependencies(self, mock_lib_preloaded): + """Test that jobs have proper dependencies""" + mock_lib_preloaded.return_value = True + + pipeline = self.create_basic_dnascope_pipeline() + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Extract all jobs + all_jobs = list(dag.waiting_jobs.keys()) + list(dag.ready_jobs.keys()) + + # Should have some ready jobs (no dependencies) + assert len(dag.ready_jobs) > 0 + + # Check that waiting jobs have valid dependencies + for job, dependencies in dag.waiting_jobs.items(): + for dep in dependencies: + assert dep in all_jobs, f"Dependency {dep} not found in job list" + + @patch('sentieon_cli.util.library_preloaded') + def test_conditional_job_creation(self, mock_lib_preloaded): + """Test that conditional jobs are created based on parameters""" + mock_lib_preloaded.return_value = True + + # Test with metrics enabled + pipeline1 = self.create_basic_dnascope_pipeline() + pipeline1.skip_metrics = False + pipeline1.validate() + pipeline1.configure() + dag1 = pipeline1.build_dag() + + # Test with metrics disabled + pipeline2 = self.create_basic_dnascope_pipeline() + pipeline2.skip_metrics = True + pipeline2.validate() + pipeline2.configure() + dag2 = pipeline2.build_dag() + + # Should have different numbers of jobs + total_jobs1 = (len(dag1.waiting_jobs) + + len(dag1.ready_jobs) + + len(dag1.finished_jobs)) + total_jobs2 = (len(dag2.waiting_jobs) + + len(dag2.ready_jobs) + + len(dag2.finished_jobs)) + + # Pipeline with metrics should have more jobs + assert total_jobs1 > total_jobs2 + + @patch('sentieon_cli.util.library_preloaded') + def test_skip_small_variants_dag(self, mock_lib_preloaded): + """Test DAG when small variant calling is skipped""" + mock_lib_preloaded.return_value = True + + pipeline = self.create_basic_dnascope_pipeline() + pipeline.skip_small_variants = True + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Extract job names + all_jobs = list(dag.waiting_jobs.keys()) + list(dag.ready_jobs.keys()) + job_names = [job.name for job in all_jobs] + + # Should not have variant calling jobs + variant_job_keywords = ["variant-calling", "model-apply", "gvcftyper"] + for keyword in variant_job_keywords: + assert not any(keyword in name for name in job_names) + + @patch('sentieon_cli.util.library_preloaded') + def test_gvcf_mode_dag_differences(self, mock_lib_preloaded): + """Test DAG differences when gVCF mode is enabled""" + mock_lib_preloaded.return_value = True + + # Test without gVCF + pipeline1 = self.create_basic_dnascope_pipeline() + pipeline1.gvcf = False + pipeline1.validate() + pipeline1.configure() + dag1 = pipeline1.build_dag() + + # Test with gVCF + pipeline2 = self.create_basic_dnascope_pipeline() + pipeline2.gvcf = True + pipeline2.validate() + pipeline2.configure() + dag2 = pipeline2.build_dag() + + # Extract job names + jobs1 = list(dag1.waiting_jobs.keys()) + list(dag1.ready_jobs.keys()) + jobs2 = list(dag2.waiting_jobs.keys()) + list(dag2.ready_jobs.keys()) + job_names1 = [job.name for job in jobs1] + job_names2 = [job.name for job in jobs2] + + # gVCF mode should have gvcftyper job + assert not any("gvcftyper" in name for name in job_names1) + assert any("gvcftyper" in name for name in job_names2) + + +class TestDAGJobProperties: + """Test properties of individual jobs in the DAG""" + + def test_job_thread_allocation(self): + """Test that jobs have appropriate thread allocation""" + # Test high-CPU jobs get multiple threads + job = Job("sentieon driver --algo DNAscope", "variant-calling", 8) + assert job.threads == 8 + + # Test lightweight jobs get fewer threads + job = Job("rm temp_file.vcf", "cleanup", 0) + assert job.threads == 0 + + def test_job_resource_requirements(self): + """Test that jobs specify resource requirements correctly""" + # Test NUMA-aware job + job = Job("align-command", "alignment", 4, resources={"node0": 1}) + assert job.resources == {"node0": 1} + + # Test job without special resources + job = Job("simple-command", "simple", 2) + assert job.resources == {} + + def test_job_failure_tolerance(self): + """Test that cleanup jobs are marked as failure-tolerant""" + # Cleanup jobs should be failure tolerant + job = Job("rm temp_files", "cleanup", 0, fail_ok=True) + assert job.fail_ok is True + + # Critical jobs should not be failure tolerant + job = Job("sentieon driver", "variant-calling", 4, fail_ok=False) + assert job.fail_ok is False + + +class TestLongReadDAGConstruction: + """Test DAG construction for long-read pipeline""" + + def setup_method(self): + """Setup test fixtures""" + self.temp_dir = tempfile.mkdtemp() + + # Create mock files + self.mock_vcf = pathlib.Path(self.temp_dir) / "output.vcf.gz" + self.mock_ref = pathlib.Path(self.temp_dir) / "reference.fa" + self.mock_bam = pathlib.Path(self.temp_dir) / "longread.bam" + self.mock_bundle = pathlib.Path(self.temp_dir) / "model.bundle" + + # Create empty files + for file_path in [self.mock_ref, self.mock_bam, self.mock_bundle]: + file_path.touch() + + def create_basic_longread_pipeline(self): + """Create a basic long-read pipeline for testing""" + pipeline = DNAscopeLRPipeline() + + # Setup logging first + from tests.utils.test_helpers import create_mock_args + args = create_mock_args() + pipeline.setup_logging(args) + + pipeline.output_vcf = self.mock_vcf + pipeline.reference = self.mock_ref + pipeline.model_bundle = self.mock_bundle + pipeline.sample_input = [self.mock_bam] + pipeline.fastq = [] + pipeline.readgroups = [] + pipeline.cores = 2 + pipeline.dry_run = True + pipeline.skip_version_check = True + pipeline.tmp_dir = pathlib.Path(self.temp_dir) + + return pipeline + + @patch('sentieon_cli.util.library_preloaded') + def test_longread_dag_complexity(self, mock_lib_preloaded): + """Test that long-read DAG is more complex due to phasing""" + mock_lib_preloaded.return_value = True + + pipeline = self.create_basic_longread_pipeline() + pipeline.validate() + pipeline.configure() + + dag = pipeline.build_dag() + + # Long-read pipeline should have many jobs due to phasing complexity + total_jobs = (len(dag.waiting_jobs) + + len(dag.ready_jobs) + + len(dag.finished_jobs)) + + # Should have multiple phases of variant calling + assert total_jobs > 10 # Rough estimate for complex phased calling + + @patch('sentieon_cli.util.library_preloaded') + def test_ont_vs_hifi_dag_differences(self, mock_lib_preloaded): + """Test DAG differences between ONT and HiFi technologies""" + mock_lib_preloaded.return_value = True + + # Test HiFi pipeline + pipeline_hifi = self.create_basic_longread_pipeline() + pipeline_hifi.tech = "HiFi" + pipeline_hifi.validate() + pipeline_hifi.configure() + dag_hifi = pipeline_hifi.build_dag() + + # Test ONT pipeline + pipeline_ont = self.create_basic_longread_pipeline() + pipeline_ont.tech = "ONT" + pipeline_ont.validate() + pipeline_ont.configure() + dag_ont = pipeline_ont.build_dag() + + # Extract all jobs + jobs_hifi = list(dag_hifi.waiting_jobs.keys()) + list(dag_hifi.ready_jobs.keys()) + jobs_ont = list(dag_ont.waiting_jobs.keys()) + list(dag_ont.ready_jobs.keys()) + + # ONT should skip CNV calling (fewer jobs) + assert len(jobs_hifi) >= len(jobs_ont) + + +class TestDAGValidation: + """Test DAG validation and correctness""" + + def test_dag_has_no_cycles(self): + """Test that generated DAGs have no cycles""" + dag = DAG() + + # Create test jobs + job1 = Job("command1", "job1") + job2 = Job("command2", "job2") + job3 = Job("command3", "job3") + + # Add jobs with dependencies: job1 -> job2 -> job3 + dag.add_job(job1) + dag.add_job(job2, {job1}) + dag.add_job(job3, {job2}) + + # This should be a valid DAG + assert job1 in dag.ready_jobs + assert job2 in dag.waiting_jobs + assert job3 in dag.waiting_jobs + + # Dependencies should be correct + assert dag.waiting_jobs[job2] == {job1} + assert dag.waiting_jobs[job3] == {job2} + + def test_dag_execution_simulation(self): + """Test simulated DAG execution""" + dag = DAG() + + # Create test jobs + job1 = Job("command1", "job1") + job2 = Job("command2", "job2") + job3 = Job("command3", "job3") + + # Add jobs: job1 and job2 can run in parallel, then job3 + dag.add_job(job1) + dag.add_job(job2) + dag.add_job(job3, {job1, job2}) + + # Simulate execution + dag_gen = dag.update_dag() + ready_jobs = dag_gen.send(None) + + # Initially job1 and job2 should be ready + assert job1 in ready_jobs + assert job2 in ready_jobs + assert job3 not in ready_jobs + + # Complete job1 + new_ready = dag_gen.send(job1) + assert len(new_ready) == 0 # job3 still waiting for job2 + + # Complete job2 + new_ready = dag_gen.send(job2) + assert job3 in new_ready # Now job3 is ready + + +if __name__ == "__main__": + pytest.main([__file__]) diff --git a/tests/unit/test_pipeline_validation.py b/tests/unit/test_pipeline_validation.py new file mode 100644 index 0000000..d9418c8 --- /dev/null +++ b/tests/unit/test_pipeline_validation.py @@ -0,0 +1,376 @@ +""" +Unit tests for pipeline validation logic +""" + +import pathlib +import pytest +import tempfile +import sys +import os +import json +from unittest.mock import patch + +# Add the parent directory to the path so we can import sentieon_cli +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))) + +from sentieon_cli.dnascope import DNAscopePipeline +from sentieon_cli.dnascope_longread import DNAscopeLRPipeline +from sentieon_cli.dnascope_hybrid import DNAscopeHybridPipeline +from tests.utils.test_helpers import create_mock_args + + +class TestDNAscopePipelineValidation: + """Test DNAscope pipeline validation logic""" + + def setup_method(self): + """Setup test fixtures""" + self.pipeline = DNAscopePipeline() + + # Setup logging first + args = create_mock_args() + self.pipeline.setup_logging(args) + + self.temp_dir = tempfile.mkdtemp() + + # Create mock files + self.mock_vcf = pathlib.Path(self.temp_dir) / "output.vcf.gz" + self.mock_ref = pathlib.Path(self.temp_dir) / "reference.fa" + self.mock_bam = pathlib.Path(self.temp_dir) / "sample.bam" + self.mock_fastq = pathlib.Path(self.temp_dir) / "sample_R1.fastq.gz" + self.mock_bundle = pathlib.Path(self.temp_dir) / "model.bundle" + + # Create empty files + for file_path in [self.mock_ref, self.mock_bam, self.mock_fastq, self.mock_bundle]: + file_path.touch() + + def test_valid_configuration_with_bam_input(self): + """Test valid configuration with BAM input""" + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.sample_input = [self.mock_bam] + self.pipeline.sr_r1_fastq = [] + self.pipeline.sr_readgroups = [] + + # Should not raise any exceptions + try: + self.pipeline.validate() + except SystemExit: + pytest.fail("Valid configuration should not raise SystemExit") + + def test_valid_configuration_with_fastq_input(self): + """Test valid configuration with FASTQ input""" + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.sample_input = [] + self.pipeline.r1_fastq = [self.mock_fastq] + self.pipeline.readgroups = ["@RG\\tID:test\\tSM:sample"] + + # Should not raise any exceptions + try: + self.pipeline.validate() + except SystemExit: + pytest.fail("Valid configuration should not raise SystemExit") + + def test_missing_inputs_raises_error(self): + """Test that missing inputs raise an error""" + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.sample_input = [] + self.pipeline.sr_r1_fastq = [] + self.pipeline.sr_readgroups = [] + + with pytest.raises(SystemExit): + self.pipeline.validate() + + def test_invalid_output_extension_raises_error(self): + """Test that invalid output file extension raises an error""" + invalid_vcf = pathlib.Path(self.temp_dir) / "output.vcf" # Missing .gz + self.pipeline.output_vcf = invalid_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.sample_input = [self.mock_bam] + self.pipeline.sr_r1_fastq = [] + self.pipeline.sr_readgroups = [] + + with pytest.raises(SystemExit): + self.pipeline.validate() + + def test_mismatched_fastq_readgroups_raises_error(self): + """Test that mismatched FASTQ and readgroup counts raise an error""" + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.sample_input = [] + self.pipeline.sr_r1_fastq = [self.mock_fastq] + self.pipeline.sr_readgroups = [] # Empty readgroups with non-empty fastq + + with pytest.raises(SystemExit): + self.pipeline.validate() + + def test_skip_multiqc_when_skip_metrics(self): + """Test that skip_multiqc is set when skip_metrics is True""" + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.sample_input = [self.mock_bam] + self.pipeline.sr_r1_fastq = [] + self.pipeline.sr_readgroups = [] + self.pipeline.skip_metrics = True + self.pipeline.skip_multiqc = False + + self.pipeline.validate() + assert self.pipeline.skip_multiqc is True + + +class TestDNAscopeLRPipelineValidation: + """Test DNAscope LongRead pipeline validation logic""" + + def setup_method(self): + """Setup test fixtures""" + self.pipeline = DNAscopeLRPipeline() + + # Setup logging first + args = create_mock_args() + self.pipeline.setup_logging(args) + + self.temp_dir = tempfile.mkdtemp() + + # Create mock files + self.mock_vcf = pathlib.Path(self.temp_dir) / "output.vcf.gz" + self.mock_ref = pathlib.Path(self.temp_dir) / "reference.fa" + self.mock_bam = pathlib.Path(self.temp_dir) / "sample.bam" + self.mock_fastq = pathlib.Path(self.temp_dir) / "sample.fastq.gz" + self.mock_bundle = pathlib.Path(self.temp_dir) / "model.bundle" + self.mock_bed = pathlib.Path(self.temp_dir) / "regions.bed" + + # Create empty files + for file_path in [self.mock_ref, self.mock_bam, self.mock_fastq, self.mock_bundle, self.mock_bed]: + file_path.touch() + + def test_valid_configuration_with_bam_input(self): + """Test valid configuration with BAM input""" + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.sample_input = [self.mock_bam] + self.pipeline.fastq = [] + self.pipeline.readgroups = [] + + try: + self.pipeline.validate() + except SystemExit: + pytest.fail("Valid configuration should not raise SystemExit") + + def test_valid_configuration_with_fastq_input(self): + """Test valid configuration with FASTQ input""" + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.lr_aln = [] + self.pipeline.fastq = [self.mock_fastq] + self.pipeline.readgroups = ["@RG\\tID:test\\tSM:sample"] + + try: + self.pipeline.validate() + except SystemExit: + pytest.fail("Valid configuration should not raise SystemExit") + + def test_ont_technology_skips_cnv(self): + """Test that ONT technology automatically skips CNV calling""" + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.sample_input = [self.mock_bam] + self.pipeline.fastq = [] + self.pipeline.readgroups = [] + self.pipeline.tech = "ONT" + self.pipeline.skip_cnv = False + + self.pipeline.validate() + assert self.pipeline.skip_cnv is True + + def test_haploid_bed_requires_diploid_bed(self): + """Test that haploid bed requires diploid bed""" + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.lr_aln = [self.mock_bam] + self.pipeline.fastq = [] + self.pipeline.readgroups = [] + self.pipeline.haploid_bed = self.mock_bed + self.pipeline.bed = None # No diploid bed + self.pipeline.skip_small_variants = False + + with pytest.raises(SystemExit): + self.pipeline.validate() + + def test_mismatched_fastq_readgroups_raises_error(self): + """Test that mismatched FASTQ and readgroup counts raise an error""" + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.lr_aln = [] + self.pipeline.fastq = [self.mock_fastq] + self.pipeline.readgroups = [] # Empty readgroups with non-empty fastq + + with pytest.raises(SystemExit): + self.pipeline.validate() + + +class TestDNAscopeHybridPipelineValidation: + """Test DNAscope Hybrid pipeline validation logic""" + + def setup_method(self): + """Setup test fixtures""" + self.pipeline = DNAscopeHybridPipeline() + + # Setup logging first + args = create_mock_args() + self.pipeline.setup_logging(args) + + self.temp_dir = tempfile.mkdtemp() + + # Create mock files + self.mock_vcf = pathlib.Path(self.temp_dir) / "output.vcf.gz" + self.mock_ref = pathlib.Path(self.temp_dir) / "reference.fa" + self.mock_lr_bam = pathlib.Path(self.temp_dir) / "longread.bam" + self.mock_sr_bam = pathlib.Path(self.temp_dir) / "shortread.bam" + self.mock_fastq = pathlib.Path(self.temp_dir) / "sample_R1.fastq.gz" + + # Create empty files + for file_path in [self.mock_ref, self.mock_lr_bam, self.mock_sr_bam, self.mock_fastq]: + file_path.touch() + + # Create mock bundle file (content will be mocked by ar_load) + self.mock_bundle = pathlib.Path(self.temp_dir) / "model.bundle" + self.mock_bundle.write_bytes(b"mock_ar_archive") + + @patch('sentieon_cli.dnascope_hybrid.ar_load') + def test_valid_hybrid_configuration(self, mock_ar_load): + """Test valid hybrid pipeline configuration""" + # Setup ar_load mock + bundle_info = { + "longReadPlatform": "HiFi", + "shortReadPlatform": "Illumina", + "minScriptVersion": "1.0.0", + "pipeline": "DNAscope Hybrid" + } + mock_ar_load.side_effect = [ + json.dumps(bundle_info).encode(), # bundle_info.json + ["longreadsv.model", "cnv.model", "bwa.model"] # bundle members + ] + + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.lr_aln = [self.mock_lr_bam] + self.pipeline.sr_aln = [self.mock_sr_bam] + self.pipeline.sr_r1_fastq = [] + self.pipeline.sr_readgroups = [] + + try: + # Only test bundle validation since full validation requires more mocking + self.pipeline.validate_bundle() + except SystemExit: + pytest.fail("Valid bundle should not raise SystemExit") + + @patch('sentieon_cli.dnascope_hybrid.ar_load') + @patch('sentieon_cli.command_strings.get_rg_lines') + def test_missing_short_read_input_raises_error(self, mock_get_rg, mock_ar_load): + """Test that missing short-read input raises an error""" + # Setup ar_load mock + bundle_info = { + "longReadPlatform": "HiFi", + "shortReadPlatform": "Illumina", + "minScriptVersion": "1.0.0", + "pipeline": "DNAscope Hybrid" + } + mock_ar_load.side_effect = [ + json.dumps(bundle_info).encode(), # bundle_info.json + ["longreadsv.model", "cnv.model", "bwa.model"] # bundle members + ] + + # Mock readgroup lines + mock_get_rg.return_value = ["@RG\tID:lr1\tSM:sample"] + + self.pipeline.output_vcf = self.mock_vcf + self.pipeline.reference = self.mock_ref + self.pipeline.model_bundle = self.mock_bundle + self.pipeline.lr_aln = [self.mock_lr_bam] + self.pipeline.sr_aln = [] # No short-read input + self.pipeline.sr_r1_fastq = [] + self.pipeline.sr_readgroups = [] + + with pytest.raises(SystemExit): + self.pipeline.validate() + + @patch('sentieon_cli.dnascope_hybrid.ar_load') + def test_invalid_bundle_pipeline_raises_error(self, mock_ar_load): + """Test that invalid bundle pipeline type raises an error""" + # Create bundle with wrong pipeline type + bundle_info = { + "longReadPlatform": "HiFi", + "shortReadPlatform": "Illumina", + "minScriptVersion": "1.0.0", + "pipeline": "DNAscope" # Wrong pipeline + } + # Mock both calls: first for bundle_info.json, second for member list + mock_ar_load.side_effect = [ + json.dumps(bundle_info).encode(), # bundle_info.json + ["longreadsv.model", "cnv.model", "bwa.model"] # bundle members + ] + + self.pipeline.model_bundle = self.mock_bundle + + with pytest.raises(SystemExit): + self.pipeline.validate_bundle() + + +class TestPipelineConfigurationHelpers: + """Test helper methods used in pipeline configuration""" + + def test_total_input_size_calculation(self): + """Test calculation of total input file size""" + pipeline = DNAscopePipeline() + + # Setup logging first + args = create_mock_args() + pipeline.setup_logging(args) + + temp_dir = tempfile.mkdtemp() + + # Create test files with known sizes + bam_file = pathlib.Path(temp_dir) / "test.bam" + fastq_file = pathlib.Path(temp_dir) / "test.fastq.gz" + + bam_file.write_bytes(b"x" * 1000) # 1KB + fastq_file.write_bytes(b"y" * 500) # 0.5KB + + pipeline.sample_input = [bam_file] + pipeline.sr_r1_fastq = [fastq_file] + pipeline.sr_r2_fastq = [] + + total_size = pipeline.total_input_size() + assert total_size == 1500 # 1KB + 0.5KB + + def test_memory_calculation_logic(self): + """Test bwt_max_mem calculation logic""" + pipeline = DNAscopePipeline() + + # Setup logging first + args = create_mock_args() + pipeline.setup_logging(args) + + # Test with specific memory values + pipeline.bwt_max_mem = "10G" + pipeline.set_bwt_max_mem(0, 1) + + import os + assert os.environ.get("bwt_max_mem") == "10G" + + +if __name__ == "__main__": + pytest.main([__file__]) diff --git a/tests/utils/test_helpers.py b/tests/utils/test_helpers.py new file mode 100644 index 0000000..3850900 --- /dev/null +++ b/tests/utils/test_helpers.py @@ -0,0 +1,367 @@ +""" +Test helper utilities for sentieon-cli testing +""" + +import pathlib +import tempfile +import shutil +import argparse +from typing import Dict, List, Optional +from unittest.mock import patch +import sys +import os + +# Add the parent directory to the path +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))) + +from sentieon_cli.dnascope import DNAscopePipeline +from sentieon_cli.dnascope_longread import DNAscopeLRPipeline +from sentieon_cli.dnascope_hybrid import DNAscopeHybridPipeline + + +def create_mock_args(loglevel="WARNING"): + """Create a mock argparse.Namespace for pipeline testing""" + args = argparse.Namespace() + args.loglevel = loglevel + return args + + +class MockFileSystem: + """Helper class for creating mock file systems for testing""" + + def __init__(self): + self.temp_dir = tempfile.mkdtemp() + self.files = {} + + def create_file(self, path: str, content: bytes = b"", size: Optional[int] = None) -> pathlib.Path: + """Create a mock file with optional content or specific size""" + file_path = pathlib.Path(self.temp_dir) / path + file_path.parent.mkdir(parents=True, exist_ok=True) + + if size is not None: + content = b"x" * size + + file_path.write_bytes(content) + self.files[path] = file_path + return file_path + + def create_fastq(self, path: str, num_reads: int = 100) -> pathlib.Path: + """Create a mock FASTQ file with specified number of reads""" + content = "" + for i in range(num_reads): + content += f"@read_{i}\n" + content += "ATCGATCGATCGATCG\n" + content += "+\n" + content += "IIIIIIIIIIIIIIII\n" + + return self.create_file(path, content.encode()) + + def create_bam_header(self, path: str, readgroups: Optional[List[Dict[str, str]]] = None) -> pathlib.Path: + """Create a mock BAM file""" + if readgroups is None: + readgroups = [{"ID": "rg1", "SM": "sample1", "PL": "ILLUMINA"}] + + # Create minimal BAM content (not a real BAM, just for testing) + content = b"BAM_HEADER" + for rg in readgroups: + rg_line = "\t".join([f"{k}:{v}" for k, v in rg.items()]) + content += f"@RG\t{rg_line}\n".encode() + + return self.create_file(path, content) + + def create_model_bundle(self, path: str, pipeline_type: str = "DNAscope", + longread_tech: str = "HiFi", shortread_tech: str = "Illumina") -> pathlib.Path: + """Create a mock model bundle file (ar archive will be mocked)""" + # Create an empty file - the ar_load function will be mocked + bundle_path = pathlib.Path(self.temp_dir) / path + bundle_path.parent.mkdir(parents=True, exist_ok=True) + + # Just create an empty file - the actual content will be provided by mocked ar_load + bundle_path.write_bytes(b"mock_ar_archive") + + self.files[path] = bundle_path + return bundle_path + + def create_bed_file(self, path: str, regions: Optional[List[tuple]] = None) -> pathlib.Path: + """Create a mock BED file""" + if regions is None: + regions = [("chr1", 1000, 2000), ("chr2", 5000, 6000)] + + content = "" + for chrom, start, end in regions: + content += f"{chrom}\t{start}\t{end}\n" + + return self.create_file(path, content.encode()) + + def cleanup(self): + """Clean up temporary files""" + shutil.rmtree(self.temp_dir, ignore_errors=True) + + +class PipelineTestHelper: + """Helper class for creating and configuring test pipelines""" + + def __init__(self): + self.fs = MockFileSystem() + + def create_dnascope_pipeline(self, **kwargs) -> DNAscopePipeline: + """Create a DNAscope pipeline with sensible defaults""" + pipeline = DNAscopePipeline() + + # Setup logging first + args = create_mock_args() + pipeline.setup_logging(args) + + # Set default files + pipeline.output_vcf = self.fs.create_file("output.vcf.gz") + pipeline.reference = self.fs.create_file("reference.fa") + pipeline.model_bundle = self.fs.create_model_bundle("model.bundle") + + # Create reference index + self.fs.create_file("reference.fa.fai") + + # Default configuration + pipeline.cores = 2 + pipeline.dry_run = True + pipeline.skip_version_check = True + pipeline.tmp_dir = pathlib.Path(self.fs.temp_dir) / "tmp" + pipeline.tmp_dir.mkdir(exist_ok=True) + + # Input files + if "use_bam" in kwargs and kwargs["use_bam"]: + pipeline.sample_input = [self.fs.create_bam_header("sample.bam")] + pipeline.r1_fastq = [] + pipeline.r2_fastq = [] + pipeline.readgroups = [] + else: + pipeline.sample_input = [] + pipeline.r1_fastq = [self.fs.create_fastq("sample_R1.fastq.gz")] + pipeline.r2_fastq = [self.fs.create_fastq("sample_R2.fastq.gz")] + pipeline.readgroups = ["@RG\\tID:rg1\\tSM:sample\\tPL:ILLUMINA"] + + # Override with any provided kwargs + for key, value in kwargs.items(): + if key != "use_bam" and hasattr(pipeline, key): + setattr(pipeline, key, value) + + return pipeline + + def create_longread_pipeline(self, **kwargs) -> DNAscopeLRPipeline: + """Create a DNAscope LongRead pipeline with sensible defaults""" + pipeline = DNAscopeLRPipeline() + + # Setup logging first + args = create_mock_args() + pipeline.setup_logging(args) + + # Set default files + pipeline.output_vcf = self.fs.create_file("output.vcf.gz") + pipeline.reference = self.fs.create_file("reference.fa") + pipeline.model_bundle = self.fs.create_model_bundle("model.bundle") + + # Default configuration + pipeline.cores = 2 + pipeline.dry_run = True + pipeline.skip_version_check = True + pipeline.tmp_dir = pathlib.Path(self.fs.temp_dir) / "tmp" + pipeline.tmp_dir.mkdir(exist_ok=True) + + # Input files + if "use_fastq" in kwargs and kwargs["use_fastq"]: + pipeline.sample_input = [] + pipeline.fastq = [self.fs.create_fastq("longread.fastq.gz", 50)] + pipeline.readgroups = ["@RG\\tID:lr1\\tSM:sample\\tPL:PACBIO"] + else: + pipeline.sample_input = [self.fs.create_bam_header("longread.bam")] + pipeline.fastq = [] + pipeline.readgroups = [] + + # Override with any provided kwargs + for key, value in kwargs.items(): + if key != "use_fastq" and hasattr(pipeline, key): + setattr(pipeline, key, value) + + return pipeline + + def create_hybrid_pipeline(self, **kwargs) -> DNAscopeHybridPipeline: + """Create a DNAscope Hybrid pipeline with sensible defaults""" + pipeline = DNAscopeHybridPipeline() + + # Setup logging first + args = create_mock_args() + pipeline.setup_logging(args) + + # Set default files + pipeline.output_vcf = self.fs.create_file("output.vcf.gz") + pipeline.reference = self.fs.create_file("reference.fa") + pipeline.model_bundle = self.fs.create_model_bundle("model.bundle", "DNAscope Hybrid") + + # Default configuration + pipeline.cores = 2 + pipeline.dry_run = True + pipeline.skip_version_check = True + pipeline.tmp_dir = pathlib.Path(self.fs.temp_dir) / "tmp" + pipeline.tmp_dir.mkdir(exist_ok=True) + + # Input files + pipeline.lr_aln = [self.fs.create_bam_header("longread.bam", + [{"ID": "lr1", "SM": "sample", "PL": "PACBIO"}])] + pipeline.sr_aln = [self.fs.create_bam_header("shortread.bam", + [{"ID": "sr1", "SM": "sample", "PL": "ILLUMINA"}])] + pipeline.sr_r1_fastq = [] + pipeline.sr_r2_fastq = [] + pipeline.sr_readgroups = [] + + # Override with any provided kwargs + for key, value in kwargs.items(): + if hasattr(pipeline, key): + setattr(pipeline, key, value) + + return pipeline + + def cleanup(self): + """Clean up test resources""" + self.fs.cleanup() + + +class MockExternalTools: + """Mock external tool dependencies for testing""" + + @staticmethod + def mock_version_check(): + """Mock version checking to always pass""" + return patch('sentieon_cli.util.check_version', return_value=True) + + @staticmethod + def mock_library_preloaded(): + """Mock library preloading check""" + return patch('sentieon_cli.util.library_preloaded', return_value=True) + + @staticmethod + def mock_ar_load(bundle_info=None, bundle_members=None): + """Mock ar_load function for bundle file testing""" + if bundle_info is None: + bundle_info = { + "longReadPlatform": "HiFi", + "shortReadPlatform": "Illumina", + "minScriptVersion": "1.0.0", + "pipeline": "DNAscope Hybrid" + } + + if bundle_members is None: + bundle_members = [ + "longreadsv.model", + "cnv.model", + "bwa.model", + "dnascope.model", + "diploid_model", + "haploid_model", + "gvcf_model", + "hybrid.model", + "HybridStage1.model", + "HybridStage2.model", + "HybridStage3.model" + ] + + def mock_ar_load_func(path): + # Parse the path to determine what to return + if path.endswith("/bundle_info.json"): + import json + return json.dumps(bundle_info).encode() + elif "/" in path and not path.endswith("/bundle_info.json"): + # Specific member request (path/member) + base_path, member = path.rsplit("/", 1) + if member in bundle_members: + return b"mock_model_data" + else: + raise RuntimeError(f"Member {member} not found in archive") + else: + # List all members - called with just the bundle path + return bundle_members + + return patch('sentieon_cli.archive.ar_load', side_effect=mock_ar_load_func) + + +class CommandValidator: + """Helper for validating generated commands""" + + @staticmethod + def validate_sentieon_command(command: str) -> bool: + """Validate that a sentieon command is well-formed""" + if not command.startswith("sentieon"): + return False + + parts = command.split() + if len(parts) < 2: + return False + + # Should have driver or util as second argument + if parts[1] not in ["driver", "util"]: + return False + + return True + + +class DAGAnalyzer: + """Helper for analyzing DAG structure in tests""" + + @staticmethod + def get_job_by_name(dag, name_pattern: str): + """Find a job in the DAG by name pattern""" + all_jobs = list(dag.waiting_jobs.keys()) + list(dag.ready_jobs.keys()) + + for job in all_jobs: + if name_pattern in job.name: + return job + return None + + @staticmethod + def get_jobs_by_type(dag, job_type: str): + """Get all jobs of a specific type""" + all_jobs = list(dag.waiting_jobs.keys()) + list(dag.ready_jobs.keys()) + + matching_jobs = [] + for job in all_jobs: + if job_type in job.name: + matching_jobs.append(job) + + return matching_jobs + + @staticmethod + def count_jobs_by_thread_usage(dag) -> Dict[int, int]: + """Count jobs by their thread usage""" + all_jobs = list(dag.waiting_jobs.keys()) + list(dag.ready_jobs.keys()) + + thread_counts = {} + for job in all_jobs: + threads = job.threads + thread_counts[threads] = thread_counts.get(threads, 0) + 1 + + return thread_counts + + +# Convenience function for common test setup +def setup_basic_test_environment(): + """Setup a basic test environment with common mocks""" + helper = PipelineTestHelper() + + patches = [ + MockExternalTools.mock_version_check(), + MockExternalTools.mock_library_preloaded(), + MockExternalTools.mock_ar_load(), + ] + + # Start all patches + started_patches = [p.start() for p in patches] + + return helper, started_patches + + +def teardown_test_environment(helper: PipelineTestHelper, patches: List): + """Teardown test environment""" + # Stop all patches + for patch in patches: + patch.stop() + + # Cleanup filesystem + helper.cleanup() From 312698bd60ce4c9a430e27853648943a62ba6c2d Mon Sep 17 00:00:00 2001 From: Don Freed Date: Sat, 7 Jun 2025 11:02:10 -0700 Subject: [PATCH 7/9] Remove unused patch functions from the test helper --- tests/integration/test_pipeline_execution.py | 16 ++--- tests/utils/test_helpers.py | 76 +------------------- 2 files changed, 10 insertions(+), 82 deletions(-) diff --git a/tests/integration/test_pipeline_execution.py b/tests/integration/test_pipeline_execution.py index f36652c..774be27 100644 --- a/tests/integration/test_pipeline_execution.py +++ b/tests/integration/test_pipeline_execution.py @@ -23,11 +23,11 @@ class TestPipelineDryRunExecution: def setup_method(self): """Setup test environment""" - self.helper, self.patches = setup_basic_test_environment() + self.helper = setup_basic_test_environment() def teardown_method(self): """Cleanup test environment""" - teardown_test_environment(self.helper, self.patches) + teardown_test_environment(self.helper) def test_dnascope_dry_run_execution(self): """Test DNAscope pipeline dry-run execution""" @@ -106,11 +106,11 @@ class TestPipelineCommandGeneration: def setup_method(self): """Setup test environment""" - self.helper, self.patches = setup_basic_test_environment() + self.helper = setup_basic_test_environment() def teardown_method(self): """Cleanup test environment""" - teardown_test_environment(self.helper, self.patches) + teardown_test_environment(self.helper) def test_dnascope_command_parameters(self): """Test DNAscope command parameter generation""" @@ -191,11 +191,11 @@ class TestPipelineErrorHandling: def setup_method(self): """Setup test environment""" - self.helper, self.patches = setup_basic_test_environment() + self.helper = setup_basic_test_environment() def teardown_method(self): """Cleanup test environment""" - teardown_test_environment(self.helper, self.patches) + teardown_test_environment(self.helper) def test_missing_required_files(self): """Test handling of missing required files""" @@ -249,11 +249,11 @@ class TestDAGDependencyValidation: def setup_method(self): """Setup test environment""" - self.helper, self.patches = setup_basic_test_environment() + self.helper = setup_basic_test_environment() def teardown_method(self): """Cleanup test environment""" - teardown_test_environment(self.helper, self.patches) + teardown_test_environment(self.helper) def test_alignment_to_dedup_dependency(self): """Test that deduplication depends on alignment""" diff --git a/tests/utils/test_helpers.py b/tests/utils/test_helpers.py index 3850900..14e47a2 100644 --- a/tests/utils/test_helpers.py +++ b/tests/utils/test_helpers.py @@ -224,64 +224,6 @@ def cleanup(self): self.fs.cleanup() -class MockExternalTools: - """Mock external tool dependencies for testing""" - - @staticmethod - def mock_version_check(): - """Mock version checking to always pass""" - return patch('sentieon_cli.util.check_version', return_value=True) - - @staticmethod - def mock_library_preloaded(): - """Mock library preloading check""" - return patch('sentieon_cli.util.library_preloaded', return_value=True) - - @staticmethod - def mock_ar_load(bundle_info=None, bundle_members=None): - """Mock ar_load function for bundle file testing""" - if bundle_info is None: - bundle_info = { - "longReadPlatform": "HiFi", - "shortReadPlatform": "Illumina", - "minScriptVersion": "1.0.0", - "pipeline": "DNAscope Hybrid" - } - - if bundle_members is None: - bundle_members = [ - "longreadsv.model", - "cnv.model", - "bwa.model", - "dnascope.model", - "diploid_model", - "haploid_model", - "gvcf_model", - "hybrid.model", - "HybridStage1.model", - "HybridStage2.model", - "HybridStage3.model" - ] - - def mock_ar_load_func(path): - # Parse the path to determine what to return - if path.endswith("/bundle_info.json"): - import json - return json.dumps(bundle_info).encode() - elif "/" in path and not path.endswith("/bundle_info.json"): - # Specific member request (path/member) - base_path, member = path.rsplit("/", 1) - if member in bundle_members: - return b"mock_model_data" - else: - raise RuntimeError(f"Member {member} not found in archive") - else: - # List all members - called with just the bundle path - return bundle_members - - return patch('sentieon_cli.archive.ar_load', side_effect=mock_ar_load_func) - - class CommandValidator: """Helper for validating generated commands""" @@ -344,24 +286,10 @@ def count_jobs_by_thread_usage(dag) -> Dict[int, int]: def setup_basic_test_environment(): """Setup a basic test environment with common mocks""" helper = PipelineTestHelper() + return helper - patches = [ - MockExternalTools.mock_version_check(), - MockExternalTools.mock_library_preloaded(), - MockExternalTools.mock_ar_load(), - ] - - # Start all patches - started_patches = [p.start() for p in patches] - return helper, started_patches - - -def teardown_test_environment(helper: PipelineTestHelper, patches: List): +def teardown_test_environment(helper: PipelineTestHelper): """Teardown test environment""" - # Stop all patches - for patch in patches: - patch.stop() - # Cleanup filesystem helper.cleanup() From 4265e7dc9a0f9ba0b2de3597edcc800cabc0e74c Mon Sep 17 00:00:00 2001 From: Don Freed Date: Tue, 17 Jun 2025 16:30:36 -0700 Subject: [PATCH 8/9] Add a hidden argument to skip the final model apply --- sentieon_cli/dnascope_hybrid.py | 39 +++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/sentieon_cli/dnascope_hybrid.py b/sentieon_cli/dnascope_hybrid.py index e44e840..b9f25c3 100644 --- a/sentieon_cli/dnascope_hybrid.py +++ b/sentieon_cli/dnascope_hybrid.py @@ -216,6 +216,10 @@ class DNAscopeHybridPipeline(DNAscopePipeline, DNAscopeLRPipeline): "help": argparse.SUPPRESS, "action": "store_true", }, + "skip_model_apply": { + "help": argparse.SUPPRESS, + "action": "store_true", + }, "sr_read_filter": { "help": argparse.SUPPRESS, }, @@ -249,6 +253,7 @@ def __init__(self) -> None: self.sr_read_filter: Optional[str] = None self.lr_read_filter: Optional[str] = None self.assay = "WGS" + self.skip_model_apply = False def validate(self) -> None: self.validate_bundle() @@ -539,8 +544,10 @@ def build_dag(self) -> DAG: dag.add_job(subset_job, {second_stage_job}) dag.add_job(concat_job, {subset_job, call2_job}) dag.add_job(anno_job, {concat_job}) - dag.add_job(apply_job, {anno_job}) - dag.add_job(norm_job, {apply_job}) + if apply_job: + dag.add_job(apply_job, {anno_job}) + if apply_job and norm_job: + dag.add_job(norm_job, {apply_job}) # Remove intermediate files during processing if not self.retain_tmpdir: @@ -575,8 +582,8 @@ def call_variants( Job, Job, Job, - Job, - Job, + Optional[Job], + Optional[Job], ]: """ Call SNVs and indels using the DNAscope hybrid pipeline @@ -895,6 +902,8 @@ def call_variants( str(files("sentieon_cli.scripts").joinpath("hybrid_anno.py")) ) combined_anno_vcf = self.tmp_dir.joinpath("combined_tmp_anno.vcf.gz") + if self.skip_model_apply: + combined_anno_vcf = self.output_vcf anno_job = Job( cmds.cmd_pyexec_hybrid_anno( combined_anno_vcf, @@ -906,6 +915,28 @@ def call_variants( "anno-calls", 0, ) + if self.skip_model_apply: + return ( + call_job, + select_job, + mapq0_job, + mapq0_slop_job, + cat_merge_job, + rm_job1, + stage1_job, + rm_job2, + second_stage_job, + rm_job3, + third_stage_job, + rm_job4, + call2_job, + subset_job, + concat_job, + rm_job5, + anno_job, + None, + None, + ) # Model Apply apply_vcf = self.tmp_dir.joinpath("combined_apply.vcf.gz") From b79ca248262297626abff6edd92619dbbc35df80 Mon Sep 17 00:00:00 2001 From: Don Freed Date: Tue, 22 Jul 2025 10:05:54 -0700 Subject: [PATCH 9/9] Remove the `tr_read_filter` and require a minimum bundle version Update the version string --- pyproject.toml | 2 +- sentieon_cli/dnascope_hybrid.py | 27 ++++++++++++++++----------- sentieon_cli/util.py | 2 +- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index bc65b04..0a2dd57 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -2,7 +2,7 @@ #https://stackoverflow.com/questions/75408641/whats-difference-between-tool-poetry-and-project-in-pyproject-toml [tool.poetry] name = "sentieon_cli" -version = "2.0.0" +version = "1.3.0" description = "entry point for sentieon command-line tools" authors = ["Don Freed ", "Brent "] readme = "README.md" diff --git a/sentieon_cli/dnascope_hybrid.py b/sentieon_cli/dnascope_hybrid.py index b9f25c3..71635b9 100644 --- a/sentieon_cli/dnascope_hybrid.py +++ b/sentieon_cli/dnascope_hybrid.py @@ -47,6 +47,10 @@ "samtools": packaging.version.Version("1.16"), } +MIN_BUNDLE_VERSION = { + "ONT": packaging.version.Version("1.2"), +} + class DNAscopeHybridPipeline(DNAscopePipeline, DNAscopeLRPipeline): """The DNAscope Hybrid pipeline""" @@ -320,6 +324,16 @@ def validate_bundle(self) -> None: if bundle_info.get("pipeline", "DNAscope Hybrid") != "DNAscope Hybrid": self.logger.error("The model bundle is for a different pipeline.") sys.exit(2) + bundle_version = packaging.version.Version( + bundle_info.get("bundleVersion", "1.0") + ) + if self.longread_tech.upper() == "ONT": + if bundle_version < MIN_BUNDLE_VERSION["ONT"]: + self.logger.error( + "The model bundle is for an older version of the " + "pipeline. Please update to the latest model bundle." + ) + sys.exit(2) bundle_members = set(ar_load(str(self.model_bundle))) if "longreadsv.model" not in bundle_members: @@ -600,7 +614,6 @@ def call_variants( sys.exit(2) # readgroup handling for long-reads - tr_read_filter: List[str] = [] lr_rg_read_filter: List[str] = [] replace_rg_args: Tuple[List[List[str]], List[List[str]]] = ([], []) for aln_rgs in self.lr_aln_readgroups: @@ -619,11 +632,6 @@ def call_variants( lr_rg_read_filter.append( f"{self.lr_read_filter},rgid={id}" ) - if self.longread_tech.upper() == "ONT": - tr_read_filter.append( - f"TrimRepeat,max_repeat_unit=2,min_repeat_span=6," - f"rgid={id}" - ) # readgroup handling for short-reads sr_rg_read_filter: List[str] = [] @@ -659,8 +667,7 @@ def call_variants( replace_rg=replace_rg_args[0] + replace_rg_args[1], input=lr_aln + sr_aln, interval=self.bed, - read_filter=tr_read_filter - + ultima_read_filter + read_filter=ultima_read_filter + lr_rg_read_filter + sr_rg_read_filter, ) @@ -859,9 +866,7 @@ def call_variants( replace_rg=replace_rg_args[0], input=lr_aln + [stage3_bam], interval=stage2_bed, - read_filter=tr_read_filter - + ultima_read_filter - + lr_rg_read_filter, + read_filter=ultima_read_filter + lr_rg_read_filter, ) driver.add_algo( DNAscope( diff --git a/sentieon_cli/util.py b/sentieon_cli/util.py index ba7e220..5572e20 100644 --- a/sentieon_cli/util.py +++ b/sentieon_cli/util.py @@ -17,7 +17,7 @@ from .logging import get_logger -__version__ = "2.0.0" +__version__ = "1.3.0" logger = get_logger(__name__)