Repository navigation
docs(zipper): name the tc tag in comments that still say pa - #695
Conversation
`fgumi zipper` writes the template-coordinate sort key as `tc`; it has been spelled that way since 0.2.0, when the tag was renamed to avoid colliding with bwa-mem's `pa:f` (primary-alignment score fraction). The code follows suit -- `add_template_coordinate_tags_raw` appends `SamTag::TC`, and `tc_info_from_raw` reads it -- but roughly forty doc comments, inline comments, and assertion messages in `zipper.rs` still call it the `pa` tag, including the section banner "Primary Alignment Tag (pa) Tests", which names the very tag the rename exists to avoid. Rename those to `tc`, along with the `has_pa_tag` local. The `--skip-pa-tags` alias is untouched: that flag really is spelled with `pa` and stays accepted. In `fgumi-raw-bam`, `test_dedup_pa_tag_check_fails_on_b_array` keeps its `pa` literal -- it reproduces a historical dedup bug -- but its comment claimed the tag was "produced by fgumi zipper", which stopped being true in 0.2.0. Say what it actually is, and note that the tag name is incidental to the bug: what defeats the dedup check is the `B:i` array type, which `tc` still uses. Comments and assertion messages only; no behavior change.
|
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe change replaces legacy ChangesTemplate-coordinate tag terminology
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #695 +/- ##
==========================================
- Coverage 93.95% 93.95% -0.01%
==========================================
Files 178 178
Lines 108059 108059
==========================================
- Hits 101525 101523 -2
- Misses 6534 6536 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@coderabbitai pause |
✅ Action performedReviews paused. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
fgumi zipperwrites the template-coordinate sort key astc, and has since 0.2.0, when the tag was renamed to stop colliding with bwa-mem'spa:f(primary-alignment score fraction). The code says so —add_template_coordinate_tags_rawappendsSamTag::TCandtc_info_from_rawreads it — but about forty doc comments, inline comments, and assertion messages inzipper.rsstill call it thepatag. The section banner reads "Primary Alignment Tag (pa) Tests", naming the exact tag the rename exists to avoid, and failures print messages likeSupplementary should have pa tagfor a tag no fgumi release has written in four minor versions.This renames those to
tc, plus thehas_pa_taglocal. The--skip-pa-tagsflag alias is deliberately untouched — that spelling is real and stays accepted.One site in
fgumi-raw-bamkeeps itspaliteral:test_dedup_pa_tag_check_fails_on_b_arrayreproduces a historical dedup bug and the legacy bytes are the point. Its comment claimed the tag was "produced by fgumi zipper", which stopped being true in 0.2.0; it now says what it actually is and notes that the tag name is incidental to the bug — what defeats dedup's check is theB:iarray type, whichtcstill uses.Comments and assertion messages only; no behavior change. Verified to merge cleanly with #682, which touches the help text in the same file.
Summary by CodeRabbit
Documentation
tctag.Tests