From 9a7f19803af0621dbc70fdd155978566c95a51e6 Mon Sep 17 00:00:00 2001 From: Jonathan Manning Date: Tue, 12 May 2026 18:38:48 +0100 Subject: [PATCH 1/2] fix(params): treat --outFileNamePrefix as a literal string prefix PathBuf semantics joined a bare-dot prefix like SAMPLE. as a directory component, so outputs landed at SAMPLE./Aligned.out.bam instead of SAMPLE.Aligned.out.bam. STAR concatenates the prefix straight onto each output filename and is the convention every STAR wrapper assumes. Change out_file_name_prefix to String and switch every consumer from PathBuf::push to a string concatenation. Trailing-slash prefixes (SAMPLE./) still work since "{prefix}X.bam" produces "SAMPLE./X.bam" which is a valid path; the bare-dot case now produces SAMPLE.X.bam at the top level, matching STAR. Fixes #26 Co-Authored-By: Claude --- src/chimeric/output.rs | 41 +++++++++++------- src/lib.rs | 30 ++++++------- src/params.rs | 57 +++++++++++++++++++++++-- tests/alignment_features.rs | 84 +++++++++++++++++++++++++++++++++++++ 4 files changed, 177 insertions(+), 35 deletions(-) diff --git a/src/chimeric/output.rs b/src/chimeric/output.rs index 37f22b2..33d2e60 100644 --- a/src/chimeric/output.rs +++ b/src/chimeric/output.rs @@ -22,8 +22,7 @@ impl ChimericJunctionWriter { /// /// Creates file: {prefix}Chimeric.out.junction pub fn new(prefix: &str) -> Result { - let mut path = PathBuf::from(prefix); - path.push("Chimeric.out.junction"); + let path = PathBuf::from(format!("{prefix}Chimeric.out.junction")); let file = File::create(&path).map_err(|e| Error::io(e, &path))?; @@ -277,22 +276,38 @@ mod tests { #[test] fn test_chimeric_junction_writer_creation() { let dir = tempdir().unwrap(); - let prefix = dir.path().to_str().unwrap(); + let prefix = format!("{}/", dir.path().display()); - let writer = ChimericJunctionWriter::new(prefix); + let writer = ChimericJunctionWriter::new(&prefix); assert!(writer.is_ok()); - let mut path = PathBuf::from(prefix); - path.push("Chimeric.out.junction"); + let path = PathBuf::from(format!("{prefix}Chimeric.out.junction")); assert!(path.exists()); } + #[test] + fn test_chimeric_junction_writer_bare_dot_prefix() { + let dir = tempdir().unwrap(); + let prefix = format!("{}/SAMPLE.", dir.path().display()); + + let writer = ChimericJunctionWriter::new(&prefix); + assert!(writer.is_ok()); + + let path = PathBuf::from(format!("{prefix}Chimeric.out.junction")); + assert!(path.exists(), "expected {} to exist", path.display()); + assert!( + path.file_name().unwrap().to_str().unwrap() == "SAMPLE.Chimeric.out.junction", + "expected literal concatenation, got {}", + path.display() + ); + } + #[test] fn test_write_inter_chromosomal() { let dir = tempdir().unwrap(); - let prefix = dir.path().to_str().unwrap(); + let prefix = format!("{}/", dir.path().display()); - let mut writer = ChimericJunctionWriter::new(prefix).unwrap(); + let mut writer = ChimericJunctionWriter::new(&prefix).unwrap(); // Create mock chimeric alignment (chr9 -> chr22, BCR-ABL fusion) let donor = ChimericSegment { @@ -337,8 +352,7 @@ mod tests { writer.flush().unwrap(); // Read file and verify - let mut path = PathBuf::from(prefix); - path.push("Chimeric.out.junction"); + let path = PathBuf::from(format!("{prefix}Chimeric.out.junction")); let mut content = String::new(); File::open(&path) @@ -369,9 +383,9 @@ mod tests { #[test] fn test_write_strand_break() { let dir = tempdir().unwrap(); - let prefix = dir.path().to_str().unwrap(); + let prefix = format!("{}/", dir.path().display()); - let mut writer = ChimericJunctionWriter::new(prefix).unwrap(); + let mut writer = ChimericJunctionWriter::new(&prefix).unwrap(); // Create mock chimeric alignment (same chr, opposite strands) let donor = ChimericSegment { @@ -416,8 +430,7 @@ mod tests { writer.flush().unwrap(); // Read file and verify - let mut path = PathBuf::from(prefix); - path.push("Chimeric.out.junction"); + let path = PathBuf::from(format!("{prefix}Chimeric.out.junction")); let mut content = String::new(); File::open(&path) diff --git a/src/lib.rs b/src/lib.rs index d229763..717f8ae 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -251,7 +251,7 @@ fn align_reads(params: &Parameters) -> anyhow::Result<()> { let time_finish = chrono::Local::now(); // Write Log.final.out - let log_path = params.out_file_name_prefix.join("Log.final.out"); + let log_path = params.output_path("Log.final.out"); if let Some(parent) = log_path.parent() { std::fs::create_dir_all(parent)?; } @@ -260,7 +260,7 @@ fn align_reads(params: &Parameters) -> anyhow::Result<()> { // Write ReadsPerGene.out.tab if quantMode GeneCounts was requested. if let Some(ref ctx) = quant_ctx { - let quant_path = params.out_file_name_prefix.join("ReadsPerGene.out.tab"); + let quant_path = params.output_path("ReadsPerGene.out.tab"); ctx.counts.write_output(&quant_path, &ctx.gene_ann)?; info!("Wrote {}", quant_path.display()); } @@ -291,9 +291,7 @@ fn run_single_pass( // Open transcriptome BAM writer if requested. let mut tr_writer: Option = if let Some(tidx) = tr.as_ref() { - let path = params - .out_file_name_prefix - .join("Aligned.toTranscriptome.out.bam"); + let path = params.output_path("Aligned.toTranscriptome.out.bam"); info!("Writing transcriptome BAM to {}", path.display()); if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; @@ -310,7 +308,7 @@ fn run_single_pass( let is_paired = params.read_files_in.len() == 2; let mut unmapped_w1: Option = if params.out_reads_unmapped == OutReadsUnmapped::Fastx { - let path = params.out_file_name_prefix.join("Unmapped.out.mate1"); + let path = params.output_path("Unmapped.out.mate1"); info!("Writing unmapped reads to {}", path.display()); Some(UnmappedFastqWriter::create(&path)?) } else { @@ -318,7 +316,7 @@ fn run_single_pass( }; let mut unmapped_w2: Option = if params.out_reads_unmapped == OutReadsUnmapped::Fastx && is_paired { - let path = params.out_file_name_prefix.join("Unmapped.out.mate2"); + let path = params.output_path("Unmapped.out.mate2"); info!("Writing unmapped mate2 reads to {}", path.display()); Some(UnmappedFastqWriter::create(&path)?) } else { @@ -357,7 +355,7 @@ fn run_single_pass( } OutStd::None => match out_type.format { OutSamFormat::Sam => { - let output_path = params.out_file_name_prefix.join("Aligned.out.sam"); + let output_path = params.output_path("Aligned.out.sam"); info!("Writing SAM to {}", output_path.display()); if let Some(parent) = output_path.parent() { std::fs::create_dir_all(parent)?; @@ -367,11 +365,9 @@ fn run_single_pass( OutSamFormat::Bam => { let sorted = out_type.sort_order == Some(OutSamSortOrder::SortedByCoordinate); let output_path = if sorted { - params - .out_file_name_prefix - .join("Aligned.sortedByCoord.out.bam") + params.output_path("Aligned.sortedByCoord.out.bam") } else { - params.out_file_name_prefix.join("Aligned.out.bam") + params.output_path("Aligned.out.bam") }; info!("Writing BAM to {}", output_path.display()); if let Some(parent) = output_path.parent() { @@ -429,7 +425,7 @@ fn run_single_pass( } // 5. Write SJ.out.tab file - let sj_output_path = params.out_file_name_prefix.join("SJ.out.tab"); + let sj_output_path = params.output_path("SJ.out.tab"); if !sj_stats.is_empty() { info!( "Writing splice junction statistics to {}", @@ -458,7 +454,7 @@ fn run_two_pass( let (sj_stats_pass1, novel_junctions) = run_pass1(index, params)?; // Write SJ.pass1.out.tab - let pass1_path = params.out_file_name_prefix.join("SJ.pass1.out.tab"); + let pass1_path = params.output_path("SJ.pass1.out.tab"); // Create output directory if it doesn't exist if let Some(parent) = pass1_path.parent() { @@ -858,12 +854,11 @@ fn align_reads_single_end( // Create chimeric output writer if enabled let mut chimeric_writer = if params.chim_segment_min > 0 && params.chim_out_junctions() { use crate::chimeric::ChimericJunctionWriter; - let prefix = params.out_file_name_prefix.to_str().unwrap_or("."); info!( "Chimeric detection enabled (chimSegmentMin={})", params.chim_segment_min ); - Some(ChimericJunctionWriter::new(prefix)?) + Some(ChimericJunctionWriter::new(¶ms.out_file_name_prefix)?) } else { None }; @@ -1292,12 +1287,11 @@ fn align_reads_paired_end( // Create chimeric output writer if enabled let mut chimeric_writer = if params.chim_segment_min > 0 && params.chim_out_junctions() { use crate::chimeric::ChimericJunctionWriter; - let prefix = params.out_file_name_prefix.to_str().unwrap_or("."); info!( "Chimeric detection enabled (chimSegmentMin={})", params.chim_segment_min ); - Some(ChimericJunctionWriter::new(prefix)?) + Some(ChimericJunctionWriter::new(¶ms.out_file_name_prefix)?) } else { None }; diff --git a/src/params.rs b/src/params.rs index 9c124b8..5b064c3 100644 --- a/src/params.rs +++ b/src/params.rs @@ -344,7 +344,7 @@ pub struct Parameters { // ── Output ────────────────────────────────────────────────────────── /// Output file name prefix (including path) #[arg(long = "outFileNamePrefix", default_value = "./")] - pub out_file_name_prefix: PathBuf, + pub out_file_name_prefix: String, /// Output type: SAM, BAM Unsorted, BAM SortedByCoordinate, None. /// Provide as space-separated tokens, e.g. "BAM SortedByCoordinate". @@ -704,6 +704,11 @@ pub struct Parameters { } impl Parameters { + /// Build an output path by concatenating `suffix` onto `out_file_name_prefix`. + pub fn output_path(&self, suffix: &str) -> PathBuf { + PathBuf::from(format!("{}{suffix}", self.out_file_name_prefix)) + } + /// Parse the raw `--outSAMtype` tokens into a structured `OutSamType`. pub fn out_sam_type(&self) -> Result { match self @@ -1010,7 +1015,7 @@ mod tests { assert_eq!(p.read_map_number, -1); assert_eq!(p.clip5p_nbases, 0); assert_eq!(p.clip3p_nbases, 0); - assert_eq!(p.out_file_name_prefix, PathBuf::from("./")); + assert_eq!(p.out_file_name_prefix, "./"); assert_eq!(p.out_sam_type_raw, vec!["SAM".to_string()]); assert_eq!(p.out_sam_strand_field, "None"); assert_eq!(p.out_sam_attributes, vec!["Standard".to_string()]); @@ -1147,7 +1152,7 @@ mod tests { sam_type.sort_order, Some(OutSamSortOrder::SortedByCoordinate) ); - assert_eq!(p.out_file_name_prefix, PathBuf::from("/out/sample1_")); + assert_eq!(p.out_file_name_prefix, "/out/sample1_"); assert_eq!(p.out_filter_multimap_nmax, 20); assert_eq!(p.align_intron_max, 1_000_000); assert_eq!(p.sjdb_gtf_file, Some(PathBuf::from("gencode.gtf"))); @@ -1488,4 +1493,50 @@ mod tests { ]); assert_eq!(p.align_sj_stitch_mismatch_nmax, vec![1, -1, 2, 3]); } + + #[test] + fn output_path_bare_dot_prefix() { + let p = parse(&["--readFilesIn", "r.fq", "--outFileNamePrefix", "SAMPLE."]); + assert_eq!(p.out_file_name_prefix, "SAMPLE."); + assert_eq!( + p.output_path("Aligned.out.bam"), + PathBuf::from("SAMPLE.Aligned.out.bam") + ); + assert_eq!( + p.output_path("Log.final.out"), + PathBuf::from("SAMPLE.Log.final.out") + ); + } + + #[test] + fn output_path_trailing_slash_prefix() { + let p = parse(&["--readFilesIn", "r.fq", "--outFileNamePrefix", "out/"]); + assert_eq!( + p.output_path("Aligned.out.bam"), + PathBuf::from("out/Aligned.out.bam") + ); + } + + #[test] + fn output_path_default_prefix() { + let p = parse(&["--readFilesIn", "r.fq"]); + assert_eq!( + p.output_path("Aligned.out.bam"), + PathBuf::from("./Aligned.out.bam") + ); + } + + #[test] + fn output_path_path_with_underscore() { + let p = parse(&[ + "--readFilesIn", + "r.fq", + "--outFileNamePrefix", + "/out/sample1_", + ]); + assert_eq!( + p.output_path("Aligned.out.bam"), + PathBuf::from("/out/sample1_Aligned.out.bam") + ); + } } diff --git a/tests/alignment_features.rs b/tests/alignment_features.rs index cc3aa78..c60acbd 100644 --- a/tests/alignment_features.rs +++ b/tests/alignment_features.rs @@ -799,3 +799,87 @@ fn test_two_pass_mode() { "expected at least 1 alignment record, got {record_count}" ); } + +// --------------------------------------------------------------------------- +// Test 9 — bare-dot prefix is treated as a literal string prefix (issue #26) +// +// STAR treats `--outFileNamePrefix SAMPLE.` as a literal prefix concatenated +// onto each output filename (SAMPLE.Aligned.out.bam at the top level), not as +// a directory name. This test asserts rustar-aligner matches that behaviour. +// --------------------------------------------------------------------------- + +#[test] +fn test_bare_dot_prefix_is_literal_string() { + let tmpdir = TempDir::new().unwrap(); + let genome = build_genome(); + let fasta = write_fasta(&tmpdir, &genome); + + let genome_dir = tmpdir.path().join("genome"); + build_index(&fasta, &genome_dir, "7", None); + + let fastq_path = tmpdir.path().join("reads.fq"); + { + let mut f = fs::File::create(&fastq_path).unwrap(); + for i in 0..50usize { + let start = 100 + i * 100; + let seq = &genome[start..start + 50]; + writeln!(f, "@read{}", i + 1).unwrap(); + f.write_all(seq).unwrap(); + writeln!(f).unwrap(); + writeln!(f, "+").unwrap(); + writeln!(f, "{}", "I".repeat(50)).unwrap(); + } + } + + let run_dir = tmpdir.path().join("bare_dot_run"); + fs::create_dir_all(&run_dir).unwrap(); + // SAMPLE. is a bare-dot prefix; STAR writes SAMPLE.Aligned.out.bam at the top level. + let prefix = format!("{}/SAMPLE.", run_dir.display()); + + Command::cargo_bin("rustar-aligner") + .unwrap() + .args([ + "--runMode", + "alignReads", + "--genomeDir", + genome_dir.to_str().unwrap(), + "--readFilesIn", + fastq_path.to_str().unwrap(), + "--outSAMtype", + "BAM", + "Unsorted", + "--outFileNamePrefix", + &prefix, + ]) + .assert() + .success(); + + let bam_path = run_dir.join("SAMPLE.Aligned.out.bam"); + let log_path = run_dir.join("SAMPLE.Log.final.out"); + let bam_as_dir = run_dir.join("SAMPLE.").join("Aligned.out.bam"); + + assert!( + bam_path.exists(), + "expected literal prefix file at {}, but it was not created", + bam_path.display() + ); + assert!( + log_path.exists(), + "expected literal prefix file at {}, but it was not created", + log_path.display() + ); + assert!( + !bam_as_dir.exists(), + "bare-dot prefix was treated as a directory: {} should not exist", + bam_as_dir.display() + ); + + let mut reader = bam::io::Reader::new(fs::File::open(&bam_path).unwrap()); + let _header = reader.read_header().expect("BAM header readable"); + let mut count = 0usize; + for rec in reader.records() { + rec.expect("valid BAM record"); + count += 1; + } + assert!(count >= 1, "expected at least 1 BAM record, got {count}"); +} From 87f67fcca2640c42a729f8a82d252e7a29d9854c Mon Sep 17 00:00:00 2001 From: "Philipp A." Date: Sat, 16 May 2026 11:46:15 +0200 Subject: [PATCH 2/2] fix merge --- tests/alignment_features.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/alignment_features.rs b/tests/alignment_features.rs index 4446375..a80c17d 100644 --- a/tests/alignment_features.rs +++ b/tests/alignment_features.rs @@ -828,8 +828,7 @@ fn test_bare_dot_prefix_is_literal_string() { // SAMPLE. is a bare-dot prefix; STAR writes SAMPLE.Aligned.out.bam at the top level. let prefix = format!("{}/SAMPLE.", run_dir.display()); - Command::cargo_bin("rustar-aligner") - .unwrap() + cargo_bin_cmd!("rustar-aligner") .args([ "--runMode", "alignReads",