Skip to content

Dep/update dependencies - #106

Merged
noamteyssier merged 4 commits into
dev-0.10.0from
dep/update-dependencies
Aug 26, 2026
Merged

noamteyssier merged 4 commits into
dev-0.10.0from
dep/update-dependencies

Conversation

@noamteyssier

Copy link
Copy Markdown
Contributor

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces an option to control whether output records are ordered to match the input order during parallel FASTX encoding. It adds an ordered flag to FastxEncoderBuilder and Encoder, updates the auto-write example with an --unordered flag, and updates dependencies. The feedback suggests using the tempfile crate in the new integration test to ensure robust cleanup of temporary files instead of manual creation and deletion.

Comment thread src/utils/fastx.rs Outdated
Comment on lines +421 to +440
let pid = std::process::id();
let input_path = std::env::temp_dir().join(format!("binseq_ordered_input_{pid}.fastq"));
let output_path = std::env::temp_dir().join(format!("binseq_ordered_output_{pid}.cbq"));
std::fs::write(&input_path, &fastq).unwrap();

let builder = BinseqWriterBuilder::new(Format::Cbq).headers(true);
let handle = Box::new(std::fs::File::create(&output_path).unwrap());
let result = FastxEncoderBuilder::new(builder, handle)
.input(&input_path)
.threads(4)
.ordered(true)
.run();
std::fs::remove_file(&input_path).unwrap();
assert!(result.is_ok());

let reader = BinseqReader::new(&output_path).unwrap();
let processor = HeaderCollector::default();
let headers = processor.headers.clone();
reader.process_parallel(processor, 4).unwrap();
std::fs::remove_file(&output_path).unwrap();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of manually creating temporary files in std::env::temp_dir() and manually cleaning them up with std::fs::remove_file, use the tempfile crate (which is already a dev-dependency). This ensures that temporary files and directories are automatically cleaned up even if the test panics or fails, preventing leftover files in the system's temporary directory.

Suggested change
let pid = std::process::id();
let input_path = std::env::temp_dir().join(format!("binseq_ordered_input_{pid}.fastq"));
let output_path = std::env::temp_dir().join(format!("binseq_ordered_output_{pid}.cbq"));
std::fs::write(&input_path, &fastq).unwrap();
let builder = BinseqWriterBuilder::new(Format::Cbq).headers(true);
let handle = Box::new(std::fs::File::create(&output_path).unwrap());
let result = FastxEncoderBuilder::new(builder, handle)
.input(&input_path)
.threads(4)
.ordered(true)
.run();
std::fs::remove_file(&input_path).unwrap();
assert!(result.is_ok());
let reader = BinseqReader::new(&output_path).unwrap();
let processor = HeaderCollector::default();
let headers = processor.headers.clone();
reader.process_parallel(processor, 4).unwrap();
std::fs::remove_file(&output_path).unwrap();
let temp_dir = tempfile::tempdir().unwrap();
let input_path = temp_dir.path().join("input.fastq");
let output_path = temp_dir.path().join("output.cbq");
std::fs::write(&input_path, &fastq).unwrap();
let builder = BinseqWriterBuilder::new(Format::Cbq).headers(true);
let handle = Box::new(std::fs::File::create(&output_path).unwrap());
let result = FastxEncoderBuilder::new(builder, handle)
.input(&input_path)
.threads(4)
.ordered(true)
.run();
assert!(result.is_ok());
let reader = BinseqReader::new(&output_path).unwrap();
let processor = HeaderCollector::default();
let headers = processor.headers.clone();
reader.process_parallel(processor, 4).unwrap();

@noamteyssier
noamteyssier merged commit 7343d5b into dev-0.10.0 Aug 26, 2026
@noamteyssier
noamteyssier deleted the dep/update-dependencies branch August 26, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant