Skip to content

Reject incomplete final entries in FASTQ streaming reads - #1035

Open
sixty-66 wants to merge 2 commits into
biotite-dev:mainfrom
sixty-66:fix/fastq-read-iter-truncated
Open

sixty-66 wants to merge 2 commits into
biotite-dev:mainfrom
sixty-66:fix/fastq-read-iter-truncated

Conversation

@sixty-66

Copy link
Copy Markdown

FastqFile.read_iter() silently stops when the final FASTQ entry ends in its sequence or quality section. For example, @read\nACGT\n+\n!! produces no entries instead of reporting the truncated quality scores, while FastqFile.read() raises InvalidFileError for the same input.

Check the parser state when the iterator reaches EOF and raise the same incomplete-entry error as the non-streaming reader. The regression test covers a header without sequence data, a missing separator, absent quality scores, and truncated quality scores, both alone and after a complete entry, for both readers.

Validation on Windows with CPython 3.12.12 and NumPy 2.5.4 (PYTHONPATH pointed at this checkout's src):

  • python -m pytest tests/sequence/test_fasta.py tests/sequence/test_fastq.py -p no:cacheprovider -q — 92 passed.
  • ruff check src/biotite/sequence/io/fastq/file.py tests/sequence/test_fastq.py — passed with Ruff 0.9.7.
  • ruff format --check src/biotite/sequence/io/fastq/file.py tests/sequence/test_fastq.py — passed.

Before the fix, the new regression cases produced 8 failures for read_iter() and 8 passes for read(). The checkout's Python modules were tested with the native Rust extension from the official Biotite 2.0.0 cp312-win_amd64 wheel. A full source build and the full test suite were not run.

@padix-key padix-key left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the fix, I only have one minor comment.

"incomplete_entry",
["@seq", "@seq\nACGT", "@seq\nACGT\n+", "@seq\nACGT\n+\n!!"],
)
def test_incomplete_last_entry(reader, prefix, incomplete_entry):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you add a docstring for this test?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the review and the positive feedback! Added the requested docstring to test_incomplete_last_entry() to explain that both readers reject incomplete final FASTQ entries, with or without a preceding complete entry.

Validation: 92 FASTA/FASTQ tests passed; Ruff lint and formatting checks passed.

This branch has not been deployed

No deployments
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.

2 participants