Skip to content

Fix parser/CLI bugs, make the docs runnable for newcomers, strengthen CI - #1

Merged
halireena merged 4 commits into
mainfrom
claude/awesome-pascal-u3xl7r
Oct 5, 2026
Merged

Fix parser/CLI bugs, make the docs runnable for newcomers, strengthen CI#1
halireena merged 4 commits into
mainfrom
claude/awesome-pascal-u3xl7r

Conversation

@halireena

@halireena halireena commented Oct 5, 2026

Copy link
Copy Markdown
Owner

Summary

A review of the R and Python packages, then a walkthrough as a new student following the READMEs from a fresh clone. This PR fixes the bugs found and every place the student got stuck.

Bug fixes (each with a test)

Python

  • read_fasta / read_fastq: a bare > or @ header raised an uncaught IndexError, which the CLI showed as a traceback. It is now a clear ValueError. FASTQ reading also tolerates blank lines between records.
  • rushtools gc aborted the whole file on an all-N record. It now prints NA for that record and continues.
  • rushtools gc --digits -1 silently printed NA for every record. It now gives an error.
  • rushtools stats on an empty file said max() arg is an empty sequence. It now says "no sequences found".
  • rushtools revcomp HELLO printed OLLED. It now rejects non-DNA characters.

R

  • count_motif(x, "") returned nchar(x). It now errors.
  • reverse_complement(NA) returned the string "NA". It now returns NA.

Behaviour changes (Python version bumped to 0.2.0)

  • gc_content(ignore_n=TRUE) now drops every non-ACGT symbol from the denominator, not just N. This matches the R package.
  • kmer_count is now case-insensitive and by default skips k-mers that contain ambiguous bases. Pass skip_ambiguous=False for the old counting.
  • R reverse_complement() now accepts IUPAC ambiguity codes, like Python.
  • Python __version__ is now read from the package metadata.

Docs

  • Added example data in examples/, so every README example runs as written. Real output is shown under each example.
  • Added a glossary, prerequisites, venv steps and a troubleshooting table.
  • Fixed placeholder install URLs: USER in python/README.md and yourGitHubName in r/README.md.
  • Added CITATION.cff, CONTRIBUTING.md and a Python changelog.

CI

  • Workflows run on pull_request as well as push.
  • Python is tested on 3.10–3.13, with ruff check, and doctests now run.
  • R runs a full R CMD check instead of devtools::test().

Testing

  • Python: pytest gives 42 passed, and ruff check . is clean.
  • R: testthat::test_local(".") passes.
  • R CMD check --as-cran locally shows only a locale warning and a clock note, both from the sandbox.
  • Every README code block was re-run from a fresh clone and compared with the output shown in the docs.

After merging

Tag a release (e.g. v0.2.0). Until this is merged, pip install and install_github from GitHub install the old code.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WqrAUWjyEmFz36zphqfoBs

Generated by Claude Code

claude added 4 commits October 5, 2026 08:58
@claude
Fix parser/CLI edge cases, align Python with R, strengthen CI
1788c4a 
Python:
- read_fasta/read_fastq: a bare ">" or "@" header raised an uncaught
  IndexError (traceback from the CLI); now a clear ValueError. FASTQ
  readers tolerate blank lines between records.
- CLI `gc` printed NA-less output and aborted the whole file on an all-N
  record; it now prints NA for that record and continues.
- CLI `stats` on an empty FASTA reported "max() arg is an empty
  sequence"; now "no sequences found".
- gc_content(ignore_n=True) now drops every non-ACGT symbol from the
  denominator, matching the R package (was N only).
- kmer_count is case-insensitive and skips k-mers with ambiguous bases
  by default (skip_ambiguous=False restores the old counting).
- __version__ is read from package metadata (single source of truth);
  version bumped to 0.2.0 for the behaviour changes. Ship py.typed.
- Doctests run under pytest; ruff lint config added.

R:
- reverse_complement() accepts IUPAC ambiguity codes, like Python.
- count_motif() errors on an empty motif instead of returning nchar(seq).

CI:
- Run on pull_request as well as push.
- Python: test 3.10-3.13 (requires-python is >=3.10) and lint with ruff.
- R: full R CMD check instead of devtools::test(), so stale Rd files,
  NAMESPACE problems and broken examples are caught.

Docs: fix the placeholder install URLs in python/README.md, add CI
badges and usage examples to the top-level README.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WqrAUWjyEmFz36zphqfoBs
@claude
Fix NA in R reverse_complement() and two silent CLI failures
902fd0b 
R:
- reverse_complement(NA) returned the string "NA" instead of NA.

Python CLI:
- `rushtools gc --digits -1` printed NA for every record (the format
  error was mistaken for "no A/C/G/T bases"); negative values are now
  rejected by argparse with a clear message.
- `rushtools revcomp HELLO` printed "OLLED" without complaint; it now
  errors on characters that are not DNA/IUPAC codes. The library
  function reverse_complement() is unchanged.

Tests for each, plus a regression test that FASTQ files with Windows
line endings read cleanly. pytest now also works before installing
(pythonpath = ["src"]).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WqrAUWjyEmFz36zphqfoBs
@claude
Make the READMEs runnable for newcomers: example data, outputs, glossary
fb50ade 
- examples/: tiny made-up genes.fasta and reads.fastq, so every example
  that used to point at a non-existent "genes.fasta" now runs as written.
- Top-level README: prerequisites, "Getting started in 5 minutes" with a
  virtual environment, the real output under every command, R install
  from the clone (no extra packages needed), a glossary (FASTA, FASTQ,
  Phred, GC, IUPAC, ORF, k-mer, N50, fold change, BH/padj, ...) and a
  troubleshooting table. The Python loop example now handles the all-N
  record instead of crashing on it.
- python/README: function table, outputs, a working "branch or tag"
  install line (there is no v0.1.0 tag yet), venv steps.
- r/README: the install line pointed at "yourGitHubName/rushtools"
  without subdir = "r"; fixed. Examples for every function with
  output; how to keep DESeq2 gene IDs (row names) through
  tidy_de_table(); how to run the tests with testthat only.
- CITATION.cff, CONTRIBUTING.md, python/CHANGELOG.md.
- python/LICENSE copyright holder now matches the R licence; R version
  0.2.0.9000 to match the "development version" heading in NEWS.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WqrAUWjyEmFz36zphqfoBs
@claude
Make the test and example commands copy-pasteable from the top folder
6b424f8 
Spell out the `cd python` / `cd r` / `cd ..` steps instead of "from the
X folder", so each block runs as written straight after the previous
one; note where to get the example file if you installed without
cloning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WqrAUWjyEmFz36zphqfoBs
@halireena
halireena merged commit fc01c34 into main Oct 5, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@halireena @claude