From 7222648abe54c2fef6e6e16bb13c8d781cf5d95a Mon Sep 17 00:00:00 2001 From: Nils Homer Date: Mon, 30 Mar 2026 22:50:56 -0700 Subject: [PATCH] fix(cli): allow explicit true/false values on all boolean flags Boolean flags using `default_value` with clap's default `SetTrue` action rejected explicit values like `--flag true` or `--flag false`. This was especially broken for 5 flags defaulting to true (--output-per-base-tags, --consensus-call-overlapping-bases, --queue-memory-per-thread, --filter-by-template, --regenerate-tags) which were stuck permanently on with no way to disable them. Adds `action = ArgAction::Set, num_args = 0..=1, default_missing_value = "true"` to all 22 boolean flags, matching sopt/fgbio behavior where both `--flag` (implies true) and `--flag=false` (explicit) are accepted. Backwards compatible: existing `--flag` usage continues to work. --- src/commands/clip.rs | 164 ++++++++++----------------------- src/commands/common.rs | 76 ++++++++++++++- src/commands/compare/bams.rs | 2 +- src/commands/dedup.rs | 4 +- src/commands/downsample.rs | 2 +- src/commands/duplex_metrics.rs | 2 +- src/commands/fastq.rs | 2 +- src/commands/filter.rs | 45 ++++++++- src/commands/review.rs | 2 +- src/commands/sort.rs | 4 +- src/commands/zipper.rs | 31 ++++++- 11 files changed, 200 insertions(+), 134 deletions(-) diff --git a/src/commands/clip.rs b/src/commands/clip.rs index 5670cc39c..aac2fc1cc 100644 --- a/src/commands/clip.rs +++ b/src/commands/clip.rs @@ -65,7 +65,7 @@ done in streaming fashion with, for example: The output sort order may be specified with --sort-order. If not given, then the output will be in the same order as input. -Any existing NM, UQ and MD tags are repaired (if --regenerate-tags is specified), and mate-pair information is updated. +Any existing NM, UQ and MD tags are repaired, and mate-pair information is updated. Three clipping modes are supported: 1. `soft` - soft-clip the bases and qualities. @@ -100,22 +100,20 @@ pub struct Clip { pub sort_order: Option, /// Clip overlapping read pairs - #[arg(long = "clip-overlapping-reads", default_value = "false")] + #[arg(long = "clip-overlapping-reads", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub clip_overlapping_reads: bool, /// Clip reads that extend past their mate's start position #[arg( long = "clip-bases-past-mate", alias = "clip-extending-past-mate", - default_value = "false" + default_value = "false", + num_args = 0..=1, + default_missing_value = "true", + action = clap::ArgAction::Set, )] pub clip_extending_past_mate: bool, - /// Note: NM/UQ/MD tags are always regenerated after clipping (matching fgbio behavior) - /// This flag is kept for backwards compatibility but is ignored - #[arg(long = "regenerate-tags", default_value = "true", hide = true)] - pub regenerate_tags: bool, - /// Minimum bases to clip from 5' end of R1 #[arg(long = "read-one-five-prime", default_value = "0")] pub read_one_five_prime: usize, @@ -133,11 +131,11 @@ pub struct Clip { pub read_two_three_prime: usize, /// Upgrade existing clipping to the specified clipping mode - #[arg(short = 'H', long = "upgrade-clipping", default_value = "false")] + #[arg(short = 'H', long = "upgrade-clipping", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub upgrade_clipping: bool, /// Automatically clip extended attributes that match read length - #[arg(short = 'a', long = "auto-clip-attributes", default_value = "false")] + #[arg(short = 'a', long = "auto-clip-attributes", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub auto_clip_attributes: bool, /// Output file for clipping metrics @@ -210,7 +208,6 @@ impl Command for Clip { info!(" Clipping mode: {}", self.clipping_mode); info!(" Clip overlapping reads: {}", self.clip_overlapping_reads); info!(" Clip extending past mate: {}", self.clip_extending_past_mate); - info!(" Regenerate tags: {}", self.regenerate_tags); info!(" {}", self.threading.log_message()); let timer = OperationTimer::new("Clipping reads"); @@ -918,7 +915,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, // Always true to match Scala fgbio + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -936,7 +933,6 @@ mod tests { assert_eq!(clip.clipping_mode, "hard"); assert!(!clip.clip_overlapping_reads); assert!(!clip.clip_extending_past_mate); - assert!(clip.regenerate_tags); // Always true } #[test] @@ -950,7 +946,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 5, read_one_three_prime: 3, read_two_five_prime: 7, @@ -982,7 +978,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: true, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1013,7 +1009,7 @@ mod tests { clipping_mode: "soft-with-mask".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1044,7 +1040,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, // Always true + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1059,7 +1055,6 @@ mod tests { queue_memory: QueueMemoryOptions::default(), }; - assert!(clip.regenerate_tags); assert_eq!(clip.reference, PathBuf::from("reference.fa")); assert!(clip.auto_clip_attributes); } @@ -1075,7 +1070,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: true, - regenerate_tags: true, + read_one_five_prime: 5, read_one_three_prime: 5, read_two_five_prime: 5, @@ -1093,7 +1088,6 @@ mod tests { // All options enabled assert!(clip.clip_overlapping_reads); assert!(clip.clip_extending_past_mate); - assert!(clip.regenerate_tags); assert!(clip.upgrade_clipping); assert!(clip.auto_clip_attributes); assert!(clip.read_one_five_prime > 0); @@ -1111,7 +1105,7 @@ mod tests { clipping_mode: "soft".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1140,7 +1134,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1169,7 +1163,7 @@ mod tests { clipping_mode: "soft".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 10, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1202,7 +1196,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1233,7 +1227,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: true, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1264,7 +1258,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1295,7 +1289,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: true, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1326,7 +1320,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1359,7 +1353,7 @@ mod tests { clipping_mode: "soft-with-mask".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 5, read_one_three_prime: 5, read_two_five_prime: 5, @@ -1390,7 +1384,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 50, read_one_three_prime: 50, read_two_five_prime: 50, @@ -1423,7 +1417,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 10, read_one_three_prime: 10, read_two_five_prime: 10, @@ -1455,7 +1449,7 @@ mod tests { clipping_mode: "soft".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1479,7 +1473,7 @@ mod tests { clipping_mode: "soft-with-mask".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1503,7 +1497,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1535,7 +1529,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: true, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1564,7 +1558,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: true, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1582,37 +1576,6 @@ mod tests { assert_eq!(clip.sort_order, Some("unsorted".to_string())); } - #[test] - fn test_clip_regenerate_tags_always_true() { - // Test that regenerate_tags is always true to match Scala fgbio behavior - let clip = Clip { - io: BamIoOptions { - input: PathBuf::from("input.bam"), - output: PathBuf::from("output.bam"), - }, - reference: PathBuf::from("reference.fa"), - clipping_mode: "hard".to_string(), - clip_overlapping_reads: false, - clip_extending_past_mate: false, - regenerate_tags: true, - read_one_five_prime: 0, - read_one_three_prime: 0, - read_two_five_prime: 0, - read_two_three_prime: 0, - upgrade_clipping: false, - auto_clip_attributes: false, - metrics: None, - sort_order: None, - threading: ThreadingOptions::none(), - compression: CompressionOptions { compression_level: 1 }, - scheduler_opts: SchedulerOptions::default(), - queue_memory: QueueMemoryOptions::default(), - }; - - // regenerate_tags should always be true to match fgbio - assert!(clip.regenerate_tags); - } - #[test] fn test_clip_single_read_end_clipping() { let clip = Clip { @@ -1624,7 +1587,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1657,7 +1620,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1688,7 +1651,7 @@ mod tests { clipping_mode: "soft".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: true, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1707,37 +1670,6 @@ mod tests { assert!(clip.clip_extending_past_mate); } - #[test] - fn test_clip_no_regenerate_tags_option() { - // Test that regenerate_tags is always true (no option to disable) - let clip = Clip { - io: BamIoOptions { - input: PathBuf::from("input.bam"), - output: PathBuf::from("output.bam"), - }, - reference: PathBuf::from("reference.fa"), - clipping_mode: "hard".to_string(), - clip_overlapping_reads: true, - clip_extending_past_mate: false, - regenerate_tags: true, - read_one_five_prime: 0, - read_one_three_prime: 0, - read_two_five_prime: 0, - read_two_three_prime: 0, - upgrade_clipping: false, - auto_clip_attributes: false, - metrics: None, - sort_order: None, - threading: ThreadingOptions::none(), - compression: CompressionOptions { compression_level: 1 }, - scheduler_opts: SchedulerOptions::default(), - queue_memory: QueueMemoryOptions::default(), - }; - - // regenerate_tags must always be true (unlike Scala which had option to disable) - assert!(clip.regenerate_tags); - } - // Integration tests use anyhow::Result; use fgumi_lib::sam::SamBuilder; @@ -1788,7 +1720,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1835,7 +1767,7 @@ mod tests { clipping_mode: "soft".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1881,7 +1813,7 @@ mod tests { clipping_mode: "soft-with-mask".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -1927,7 +1859,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 3, read_one_three_prime: 2, read_two_five_prime: 2, @@ -1973,7 +1905,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: true, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -2019,7 +1951,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -2066,7 +1998,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -2113,7 +2045,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -2159,7 +2091,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 3, read_one_three_prime: 2, read_two_five_prime: 0, @@ -2206,7 +2138,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -2253,7 +2185,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: true, - regenerate_tags: true, + read_one_five_prime: 2, read_one_three_prime: 2, read_two_five_prime: 2, @@ -2300,7 +2232,7 @@ mod tests { clipping_mode: "invalid".to_string(), // Invalid mode clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -2345,7 +2277,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -2398,7 +2330,7 @@ mod tests { clipping_mode: "hard".to_string(), clip_overlapping_reads: true, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime: 0, read_one_three_prime: 0, read_two_five_prime: 0, @@ -2462,7 +2394,7 @@ mod tests { clipping_mode: "soft".to_string(), clip_overlapping_reads: false, clip_extending_past_mate: false, - regenerate_tags: true, + read_one_five_prime, read_one_three_prime, read_two_five_prime, diff --git a/src/commands/common.rs b/src/commands/common.rs index 21b297276..2f23bbe40 100644 --- a/src/commands/common.rs +++ b/src/commands/common.rs @@ -104,11 +104,11 @@ pub struct ConsensusCallingOptions { pub min_input_base_quality: u8, /// Produce per-base tags (cd, ce) in addition to per-read tags - #[arg(short = 'B', long = "output-per-base-tags", default_value = "true")] + #[arg(short = 'B', long = "output-per-base-tags", default_value = "true", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub output_per_base_tags: bool, /// Quality-trim reads before consensus calling (removes low-quality bases from ends) - #[arg(long = "trim", default_value = "false")] + #[arg(long = "trim", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub trim: bool, /// Minimum consensus base quality (output consensus bases below this are masked to N) @@ -214,7 +214,7 @@ impl Default for ReadGroupOptions { #[derive(Debug, Clone, Args)] pub struct OverlappingConsensusOptions { /// Consensus call overlapping bases in read pairs before UMI consensus calling - #[arg(long = "consensus-call-overlapping-bases", default_value = "true")] + #[arg(long = "consensus-call-overlapping-bases", default_value = "true", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub consensus_call_overlapping_bases: bool, } @@ -451,7 +451,7 @@ pub struct QueueMemoryOptions { /// When true, total memory = queue-memory * threads. For example, /// --queue-memory 768 with --threads 16 allocates 12 GB total. /// Set to false for a fixed total memory budget regardless of thread count. - #[arg(long = "queue-memory-per-thread", default_value = "true")] + #[arg(long = "queue-memory-per-thread", default_value = "true", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub queue_memory_per_thread: bool, /// DEPRECATED: Use --queue-memory instead. Memory limit for pipeline queues in megabytes. @@ -1102,4 +1102,72 @@ mod tests { assert!(available > 0); assert!(available <= total); } + + use clap::Parser; + + /// Test-only wrapper to exercise clap parsing of flattened Args structs. + #[derive(Debug, Parser)] + #[command(name = "test")] + struct TestBoolFlags { + #[command(flatten)] + consensus: ConsensusCallingOptions, + #[command(flatten)] + overlapping: OverlappingConsensusOptions, + #[command(flatten)] + queue_memory: QueueMemoryOptions, + } + + use rstest::rstest; + + #[rstest] + // --output-per-base-tags (default true) + #[case(&["test"], true)] + #[case(&["test", "--output-per-base-tags"], true)] + #[case(&["test", "--output-per-base-tags", "true"], true)] + #[case(&["test", "--output-per-base-tags", "false"], false)] + #[case(&["test", "--output-per-base-tags=true"], true)] + #[case(&["test", "--output-per-base-tags=false"], false)] + fn test_output_per_base_tags_parsing(#[case] args: &[&str], #[case] expected: bool) { + let cmd = TestBoolFlags::try_parse_from(args).unwrap(); + assert_eq!(cmd.consensus.output_per_base_tags, expected); + } + + #[rstest] + // --trim (default false) + #[case(&["test"], false)] + #[case(&["test", "--trim"], true)] + #[case(&["test", "--trim", "true"], true)] + #[case(&["test", "--trim", "false"], false)] + #[case(&["test", "--trim=true"], true)] + #[case(&["test", "--trim=false"], false)] + fn test_trim_parsing(#[case] args: &[&str], #[case] expected: bool) { + let cmd = TestBoolFlags::try_parse_from(args).unwrap(); + assert_eq!(cmd.consensus.trim, expected); + } + + #[rstest] + // --consensus-call-overlapping-bases (default true) + #[case(&["test"], true)] + #[case(&["test", "--consensus-call-overlapping-bases"], true)] + #[case(&["test", "--consensus-call-overlapping-bases", "true"], true)] + #[case(&["test", "--consensus-call-overlapping-bases", "false"], false)] + #[case(&["test", "--consensus-call-overlapping-bases=true"], true)] + #[case(&["test", "--consensus-call-overlapping-bases=false"], false)] + fn test_overlapping_bases_parsing(#[case] args: &[&str], #[case] expected: bool) { + let cmd = TestBoolFlags::try_parse_from(args).unwrap(); + assert_eq!(cmd.overlapping.consensus_call_overlapping_bases, expected); + } + + #[rstest] + // --queue-memory-per-thread (default true) + #[case(&["test"], true)] + #[case(&["test", "--queue-memory-per-thread"], true)] + #[case(&["test", "--queue-memory-per-thread", "true"], true)] + #[case(&["test", "--queue-memory-per-thread", "false"], false)] + #[case(&["test", "--queue-memory-per-thread=true"], true)] + #[case(&["test", "--queue-memory-per-thread=false"], false)] + fn test_queue_memory_per_thread_parsing(#[case] args: &[&str], #[case] expected: bool) { + let cmd = TestBoolFlags::try_parse_from(args).unwrap(); + assert_eq!(cmd.queue_memory.queue_memory_per_thread, expected); + } } diff --git a/src/commands/compare/bams.rs b/src/commands/compare/bams.rs index dbe1430e9..429f9908a 100644 --- a/src/commands/compare/bams.rs +++ b/src/commands/compare/bams.rs @@ -171,7 +171,7 @@ pub struct CompareBams { /// Required for comparing output from consensus commands (simplex/duplex/codec) /// when run with --threads, as parallel processing causes non-deterministic ordering. /// Only valid with --mode grouping. - #[arg(long = "ignore-order", default_value = "false")] + #[arg(long = "ignore-order", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub ignore_order: bool, /// Initial buffer size for --ignore-order mode (number of records) diff --git a/src/commands/dedup.rs b/src/commands/dedup.rs index 901b8c567..b11d11168 100644 --- a/src/commands/dedup.rs +++ b/src/commands/dedup.rs @@ -1073,7 +1073,7 @@ pub struct MarkDuplicates { pub family_size_histogram: Option, /// Remove duplicates instead of just marking them - #[arg(short = 'r', long = "remove-duplicates", default_value = "false")] + #[arg(short = 'r', long = "remove-duplicates", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub remove_duplicates: bool, /// The tag containing the raw UMI sequence @@ -1097,7 +1097,7 @@ pub struct MarkDuplicates { pub min_map_q: Option, /// Include reads flagged as not passing QC - #[arg(short = 'n', long = "include-non-pf-reads", default_value = "false")] + #[arg(short = 'n', long = "include-non-pf-reads", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub include_non_pf_reads: bool, /// UMI grouping strategy diff --git a/src/commands/downsample.rs b/src/commands/downsample.rs index 7a0a39107..874dd6bd1 100644 --- a/src/commands/downsample.rs +++ b/src/commands/downsample.rs @@ -76,7 +76,7 @@ pub struct Downsample { pub seed: Option, /// Validate that MI tags appear in consecutive groups (error if seen non-consecutively) - #[arg(long = "validate-mi-order", default_value = "false")] + #[arg(long = "validate-mi-order", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub validate_mi_order: bool, /// Output file for kept family size histogram diff --git a/src/commands/duplex_metrics.rs b/src/commands/duplex_metrics.rs index 47add8c03..03064f588 100644 --- a/src/commands/duplex_metrics.rs +++ b/src/commands/duplex_metrics.rs @@ -100,7 +100,7 @@ pub struct DuplexMetrics { pub min_ba_reads: usize, /// Collect duplex UMI counts (memory intensive) - #[arg(long = "duplex-umi-counts", default_value = "false")] + #[arg(long = "duplex-umi-counts", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub duplex_umi_counts: bool, /// Optional intervals file to restrict analysis (BED or Picard interval list format) diff --git a/src/commands/fastq.rs b/src/commands/fastq.rs index 2f3d9ce00..e2b685810 100644 --- a/src/commands/fastq.rs +++ b/src/commands/fastq.rs @@ -73,7 +73,7 @@ pub struct Fastq { pub input: PathBuf, /// Don't append /1 and /2 to read names. - #[arg(short = 'n', long = "no-read-suffix", default_value = "false")] + #[arg(short = 'n', long = "no-read-suffix", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub no_suffix: bool, /// Exclude reads with any of these flags present [0x900 = secondary|supplementary]. diff --git a/src/commands/filter.rs b/src/commands/filter.rs index 28d57247d..78606df8b 100644 --- a/src/commands/filter.rs +++ b/src/commands/filter.rs @@ -147,7 +147,7 @@ pub struct Filter { pub max_no_call_fraction: f64, /// Reverse per-base tags for negative strand reads - #[arg(short = 'R', long = "reverse-per-base-tags", default_value = "false")] + #[arg(short = 'R', long = "reverse-per-base-tags", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub reverse_per_base_tags: bool, /// Threading options for parallel processing @@ -155,7 +155,7 @@ pub struct Filter { pub threading: ThreadingOptions, /// Filter templates together (all primary reads must pass) - #[arg(long = "filter-by-template", default_value = "true")] + #[arg(long = "filter-by-template", default_value = "true", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub filter_by_template: bool, /// Optional output BAM file for rejected reads @@ -167,7 +167,7 @@ pub struct Filter { pub stats: Option, /// Require single-strand agreement for duplex consensus (mask bases where AB and BA disagree) - #[arg(short = 's', long = "require-single-strand-agreement", default_value = "false")] + #[arg(short = 's', long = "require-single-strand-agreement", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub require_single_strand_agreement: bool, /// Compression options for output BAM. @@ -3948,4 +3948,43 @@ mod tests { Ok(()) } + + #[rstest] + // --reverse-per-base-tags (default false) + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1"], false)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--reverse-per-base-tags"], true)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--reverse-per-base-tags", "true"], true)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--reverse-per-base-tags", "false"], false)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--reverse-per-base-tags=true"], true)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--reverse-per-base-tags=false"], false)] + fn test_reverse_per_base_tags_parsing(#[case] args: &[&str], #[case] expected: bool) { + let cmd = Filter::try_parse_from(args).unwrap(); + assert_eq!(cmd.reverse_per_base_tags, expected); + } + + #[rstest] + // --filter-by-template (default true) + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1"], true)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--filter-by-template"], true)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--filter-by-template", "true"], true)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--filter-by-template", "false"], false)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--filter-by-template=true"], true)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--filter-by-template=false"], false)] + fn test_filter_by_template_parsing(#[case] args: &[&str], #[case] expected: bool) { + let cmd = Filter::try_parse_from(args).unwrap(); + assert_eq!(cmd.filter_by_template, expected); + } + + #[rstest] + // --require-single-strand-agreement (default false) + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1"], false)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--require-single-strand-agreement"], true)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--require-single-strand-agreement", "true"], true)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--require-single-strand-agreement", "false"], false)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--require-single-strand-agreement=true"], true)] + #[case(&["filter", "-i", "in.bam", "-o", "out.bam", "-M", "1", "--require-single-strand-agreement=false"], false)] + fn test_require_single_strand_agreement_parsing(#[case] args: &[&str], #[case] expected: bool) { + let cmd = Filter::try_parse_from(args).unwrap(); + assert_eq!(cmd.require_single_strand_agreement, expected); + } } diff --git a/src/commands/review.rs b/src/commands/review.rs index ed03465ff..f7161871d 100644 --- a/src/commands/review.rs +++ b/src/commands/review.rs @@ -91,7 +91,7 @@ pub struct Review { pub sample: Option, /// Ignore N bases in consensus reads - #[arg(short = 'N', long = "ignore-ns", default_value = "false")] + #[arg(short = 'N', long = "ignore-ns", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub ignore_ns: bool, /// Only output detailed information for variants at or below this MAF diff --git a/src/commands/sort.rs b/src/commands/sort.rs index 8fc986f0c..d07f35816 100644 --- a/src/commands/sort.rs +++ b/src/commands/sort.rs @@ -140,7 +140,7 @@ pub struct Sort { /// /// When enabled (default), --max-memory specifies memory per thread. /// Total memory = `max_memory` × threads. Disable for fixed total memory. - #[arg(long = "memory-per-thread", default_value = "true", action = clap::ArgAction::Set)] + #[arg(long = "memory-per-thread", default_value = "true", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub memory_per_thread: bool, /// Temporary directory for intermediate files. @@ -174,7 +174,7 @@ pub struct Sort { /// Only valid for coordinate sort. The index file will be written to /// `.bai`. Uses single-threaded compression for accurate virtual /// position tracking. - #[arg(long = "write-index", default_value = "false")] + #[arg(long = "write-index", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub write_index: bool, /// Cell barcode tag for template-coordinate sort. diff --git a/src/commands/zipper.rs b/src/commands/zipper.rs index 38b0dfcaf..30959badb 100644 --- a/src/commands/zipper.rs +++ b/src/commands/zipper.rs @@ -165,14 +165,14 @@ pub struct Zipper { /// Exclude reads from the unmapped BAM that are not present in the aligned BAM. /// Useful when reads were intentionally removed (e.g., by adapter trimming) prior to alignment. - #[arg(long = "exclude-missing-reads", default_value = "false")] + #[arg(long = "exclude-missing-reads", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub exclude_missing_reads: bool, /// Skip adding `pa` (primary alignment) tags to secondary/supplementary reads. /// By default, zipper adds a `pa` tag containing the primary alignment's template /// sort key coordinates, which enables correct template-coordinate sorting and /// deduplication of these reads. Use this flag if you don't need this functionality. - #[arg(long = "skip-pa-tags", default_value = "false")] + #[arg(long = "skip-pa-tags", default_value = "false", num_args = 0..=1, default_missing_value = "true", action = clap::ArgAction::Set)] pub skip_pa_tags: bool, } @@ -946,6 +946,7 @@ mod tests { use noodles::sam::alignment::record::data::field::Tag; use noodles::sam::alignment::record_buf::RecordBuf; use noodles::sam::alignment::record_buf::data::field::Value as BufValue; + use rstest::rstest; use std::collections::HashMap; use tempfile::TempDir; @@ -2549,4 +2550,30 @@ mod tests { Ok(()) } + + #[rstest] + // --exclude-missing-reads (default false) + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam"], false)] + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam", "--exclude-missing-reads"], true)] + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam", "--exclude-missing-reads", "true"], true)] + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam", "--exclude-missing-reads", "false"], false)] + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam", "--exclude-missing-reads=true"], true)] + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam", "--exclude-missing-reads=false"], false)] + fn test_exclude_missing_reads_parsing(#[case] args: &[&str], #[case] expected: bool) { + let cmd = Zipper::try_parse_from(args).unwrap(); + assert_eq!(cmd.exclude_missing_reads, expected); + } + + #[rstest] + // --skip-pa-tags (default false) + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam"], false)] + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam", "--skip-pa-tags"], true)] + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam", "--skip-pa-tags", "true"], true)] + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam", "--skip-pa-tags", "false"], false)] + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam", "--skip-pa-tags=true"], true)] + #[case(&["zipper", "-u", "u.bam", "-r", "ref.fa", "-o", "out.bam", "--skip-pa-tags=false"], false)] + fn test_skip_pa_tags_parsing(#[case] args: &[&str], #[case] expected: bool) { + let cmd = Zipper::try_parse_from(args).unwrap(); + assert_eq!(cmd.skip_pa_tags, expected); + } }