Skip to content

Add FastqFile.Offset enum - #928

Merged
padix-key merged 3 commits into
biotite-dev:v2from
ehsanestaji:feat/923-fastq-offset-enum
Jul 18, 2026
Merged

Add FastqFile.Offset enum#928
padix-key merged 3 commits into
biotite-dev:v2from
ehsanestaji:feat/923-fastq-offset-enum

Conversation

@ehsanestaji

@ehsanestaji ehsanestaji commented Jul 16, 2026

Copy link
Copy Markdown

Summary

  • Add a nested FastqFile.Offset IntEnum for the supported FASTQ quality-score encodings.
  • Use the enum in the constructor, read(), read_iter(), and write_iter() type hints and documentation.
  • Update biotite.application.sra to expose the same offset API.
  • Continue accepting integer offsets while removing the legacy string aliases for Biotite v2.

Motivation

The enum makes the supported FASTQ variants discoverable while mapping directly to their numeric ASCII offsets. Since this change targets Biotite v2, the legacy string aliases are removed rather than carried forward as compatibility behavior.

Validation

  • pixi run -e test test tests/sequence/test_fastq.py -q — 23 passed
  • pixi run lint

Closes #923

@padix-key
padix-key changed the base branch from main to v2 July 16, 2026 11:39
@padix-key

Copy link
Copy Markdown
Member

Thanks for tackling the issue! This change is planned for v2 of Biotite (I also changed the target branch of this PR), so we do not need to keep the string values for compatibility reasons. Note that biotite.application.sra needs to be updated as well as it exposes the same offset values.

assert np.array_equal(test_scores, ref_scores)


@pytest.mark.parametrize(

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.

Without the need of the legacy names I think this test can be omitted.

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.

Addressed in eb4753b7 — I removed the obsolete legacy-string test. The broader update also applies FastqFile.Offset throughout biotite.application.sra. Validation: 23 FASTQ tests passed and pixi run lint completed successfully.

@codspeed-hq

codspeed-hq Bot commented Jul 16, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 13.39%

⚡ 5 improved benchmarks
✅ 98 untouched benchmarks
⏩ 14 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
benchmark_match_kmer_selection[BucketKmerTable(10000)-None] 353.3 µs 304.1 µs +16.18%
benchmark_match[BucketKmerTable(10000)-None] 401.2 µs 352.2 µs +13.91%
benchmark_match_kmer_selection[KmerTable-None] 278.3 µs 245.7 µs +13.29%
benchmark_match_kmer_selection[KmerTable-11*11*1*1***111] 275.3 µs 244.5 µs +12.58%
benchmark_match[KmerTable-None] 327 µs 294.5 µs +11.03%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ehsanestaji:feat/923-fastq-offset-enum (eb4753b) with main (1fa1b70)2

Open in CodSpeed

Footnotes

  1. 14 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on v2 (1fa1b70) during the generation of this report, so main (1fa1b70) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@ehsanestaji
ehsanestaji requested a review from padix-key July 16, 2026 22:43
@padix-key
padix-key merged commit 1b203c7 into biotite-dev:v2 Jul 18, 2026
24 checks passed
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.

Make FastqFile offset an IntEnum

2 participants