Skip to content

Conversation

@nh13
Copy link
Member

@nh13 nh13 commented Dec 16, 2025

Supersedes #161.

Adds pytest-doctestplus and fixes docstrings for doctest compatibility. Also adds a mkdocs hook to strip doctest flags from rendered docs.


📚 Documentation preview 📚: https://fgpyo--268.org.readthedocs.build/en/268/

- Add pytest-doctestplus dependency and configure pytest for doctests
- Add poe task `check-doctests` to run doctests independently
- Fix docstrings across 11 modules for doctest compatibility:
  - Unindent code examples from fenced blocks
  - Add doctest directives (SKIP, ELLIPSIS, NORMALIZE_WHITESPACE)
  - Fix expected output formatting
- Add mkdocs hook to strip doctest flags from rendered documentation
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 16, 2025

📝 Walkthrough

Walkthrough

This pull request establishes doctest management infrastructure and updates doctests across the codebase. Changes include: adding a new MkDocs hook script (docs/scripts/strip_doctest_flags.py) to strip doctest flags from rendered HTML, registering the hook in mkdocs.yml, and configuring pyproject.toml with pytest-doctestplus and doctest option flags (NORMALIZE_WHITESPACE, ELLIPSIS). Doctests are then updated across multiple fgpyo modules to use proper doctest directives (e.g., +SKIP, +ELLIPSIS), standardized formatting with explicit prompts, corrected import paths, and adjusted expected output representations. No functional code logic or public API signatures are modified.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly and concisely describes the main change: adding doctest support to pytest via pytest-doctestplus.
Description check ✅ Passed Description directly relates to the changeset, explaining pytest-doctestplus addition, docstring fixes, and the mkdocs hook for stripping doctest flags.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-doctest-support

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1ffed0 and 503b1c0.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (14)
  • docs/scripts/strip_doctest_flags.py (1 hunks)
  • fgpyo/collections/__init__.py (4 hunks)
  • fgpyo/fasta/builder.py (1 hunks)
  • fgpyo/fasta/sequence_dictionary.py (5 hunks)
  • fgpyo/fastx/__init__.py (1 hunks)
  • fgpyo/io/__init__.py (4 hunks)
  • fgpyo/read_structure.py (1 hunks)
  • fgpyo/sam/__init__.py (2 hunks)
  • fgpyo/sam/clipping.py (2 hunks)
  • fgpyo/util/logging.py (1 hunks)
  • fgpyo/util/metric.py (1 hunks)
  • fgpyo/vcf/__init__.py (1 hunks)
  • mkdocs.yml (1 hunks)
  • pyproject.toml (4 hunks)
🧰 Additional context used
🪛 Ruff (0.14.8)
docs/scripts/strip_doctest_flags.py

15-15: Unused function argument: kwargs

(ARG001)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Tests (3.13)
🔇 Additional comments (28)
fgpyo/fasta/sequence_dictionary.py (3)

45-45: LGTM.

Import of Keys is necessary for the doctest namespace.


88-90: LGTM.

Proper use of +ELLIPSIS for object representation.


12-14: The test data path ./tests/fgpyo/sam/data/valid.sam exists.

fgpyo/fastx/__init__.py (1)

18-24: LGTM!

+SKIP is correctly applied—this example requires external files that won't exist during doctest runs.

fgpyo/util/logging.py (1)

13-31: LGTM!

Doctest formatting is correct. Example is self-contained and testable.

fgpyo/collections/__init__.py (2)

38-41: LGTM!

Standard doctest exception format. The ... wildcard properly handles traceback variations.


71-74: LGTM!

Consistent exception formatting with the earlier example.

fgpyo/vcf/__init__.py (1)

22-51: LGTM!

Doctest examples properly formatted. +SKIP correctly applied to to_path() which involves filesystem operations.

fgpyo/read_structure.py (1)

18-47: LGTM!

Doctest outputs updated to match actual Python repr format. Exception handling example is valid.

mkdocs.yml (1)

15-16: LGTM!

Hook registration is correct.

pyproject.toml (5)

51-51: LGTM!

pytest-doctestplus dependency added appropriately.


127-131: LGTM!

Doctest configuration is correct. NORMALIZE_WHITESPACE and ELLIPSIS flags are appropriate for the doctest examples across the codebase.


143-148: LGTM!

check-doctests task properly defined.


150-158: LGTM!

check-doctests integrated into check-all sequence.


160-168: LGTM!

check-doctests integrated into fix-and-check-all sequence.

fgpyo/io/__init__.py (4)

17-59: LGTM!

Module doctest examples properly formatted with appropriate use of +ELLIPSIS and +SKIP flags. Use of getfixture("tmp_path") is correct for pytest-doctestplus.


186-189: LGTM!

Function doctest examples properly use +SKIP flag for I/O operations.


211-214: LGTM!

Function doctest examples properly use +SKIP flag for I/O operations.


249-250: LGTM!

Function doctest examples properly use +SKIP flag for I/O operations.

fgpyo/sam/clipping.py (2)

26-41: LGTM!

Doctest example properly demonstrates the clipping functionality with correct formatting and expected output.


50-56: LGTM!

Documentation formatting improvements are clear and correct.

fgpyo/util/metric.py (1)

24-123: LGTM!

Doctest examples reformatted correctly with proper >>> prompts, updated expected outputs, and appropriate +SKIP flags for file operations.

fgpyo/fasta/builder.py (1)

12-44: LGTM!

Doctest examples properly updated with +ELLIPSIS for object representations and +SKIP for file I/O operations. Use of getfixture("tmp_path") is correct.

fgpyo/sam/__init__.py (4)

31-79: LGTM!

SAM/BAM reading/writing doctest examples properly updated with explicit record.query_name usage and appropriate +SKIP flags.


99-113: LGTM!

Cigar parsing examples updated correctly. Error example now shows proper Traceback format with exception details.


124-149: LGTM!

Cigar element access examples correctly updated to use .elements accessor with proper expected outputs.


155-168: LGTM!

SupplementaryAlignment parsing examples properly formatted with correct expected outputs.

docs/scripts/strip_doctest_flags.py (1)

15-25: **kwargs is required by MkDocs plugin API.

MkDocs' page_content event passes html, page, config, and site_navigation parameters. The function correctly accepts all of these via **kwargs as part of the hook contract. Ruff's unused parameter warning is a false positive since MkDocs will provide these arguments at runtime.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Member

@clintval clintval left a comment

Choose a reason for hiding this comment

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

Excellent, overdue, and very welcome. Thank you!

@nh13 nh13 merged commit 8f5b384 into main Dec 17, 2025
13 checks passed
@nh13 nh13 deleted the add-doctest-support branch December 17, 2025 16:22
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.

4 participants