feat: check that all input FASTQs can be opened before counting#29
Open
werner291 wants to merge 2 commits into
Open
feat: check that all input FASTQs can be opened before counting#29werner291 wants to merge 2 commits into
werner291 wants to merge 2 commits into
Conversation
werner291
force-pushed
the
wk/check-input-fastqs-readable
branch
from
1817b7d to
609f664
Compare
Open each input FASTQ for reading up front so a missing or unreadable file is reported immediately rather than after earlier files have been processed. All inputs are checked so the user sees every problem in one error rather than fixing them one at a time. Closes fulcrumgenomics#6
werner291
force-pushed
the
wk/check-input-fastqs-readable
branch
from
609f664 to
508cfc3
Compare
nh13
requested changes
May 19, 2026
nh13
left a comment
nh13
left a comment
Member
There was a problem hiding this comment.
I appreciate you tackling this, unfortunately this will fail on named pipes and shell redirection, which will need to be tackled as part of this PR.
The pre-flight check called File::open on every input path. For stream-backed paths (FIFOs, character devices like /dev/stdin, and shell process substitution backed by /dev/fd/N pipes) opening twice either blocks waiting for a writer or consumes bytes the main read path never sees, so what was intended as a friendly early error broke perfectly valid invocations like guide-counter count -i <(zcat reads.fq.gz) ... mkfifo /tmp/p && producer > /tmp/p & guide-counter count -i /tmp/p ... Pre-flight regular files only. Classify with fs::metadata so the symlink /dev/stdin resolves to whatever stdin actually points at; non-regular inputs fall through to the main read path, which opens them exactly once. Adds tests covering a real mkfifo'd named pipe and /dev/stdin.
werner291
force-pushed
the
wk/check-input-fastqs-readable
branch
from
ad7b3d3 to
d52c13e
Compare
Author
|
Good catch. The fixup narrows the pre-flight to regular files; non-regular paths (FIFOs, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6.
Opens each input FASTQ up front and reports every unreadable path in one error, so a typo on the last file doesn't surface only after earlier files have been processed.
Count::executealso loads--libraryand optionally--essential-genes/--nonessential-genes/--control-guidesbefore the heavy work. Happy to extend the pre-flight to those too if you'd prefer one consolidated error covering all inputs.