Repository navigation
Restore archived Python bulk SNP counting and analysis compatibility - #135
Merged
Merged
Conversation
Jaureguy760
marked this pull request as ready for review
October 15, 2026 00:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bulk Rust counting scanned BAM records before choosing a SNP, whereas archived Python processes SNPs in input order and consumes each read name once per chromosome. Paired mates could therefore contribute to a different SNP, and deletions/reference skips could incorrectly leave a name eligible for a later SNP. The biallelic SNP path now preserves Python's per-SNP fetch order and seen-name timing. Existing non-SNP counting remains outside this fix.
Also restores per-SNP pseudocount removal from reported bulk region counts, fixes single-cell preparation and differential analysis for cells in obs / variants in var, and widens differential sums to UInt64. Runtime Polars and Galaxy CI dependencies are constrained to compatible versions.
Counting comparisons execute the unmodified count_snp_alleles function from a verbatim source fixture pinned to Python commit 41354fc. Cases include reversed SNP order, paired mates, multi-SNP reads, repeated read names, deletions, reference skips, overlapping region membership, chromosome separation, one/two threads, and the repository's shared BAM/VCF fixture. This replaces a prior Python reference that reproduced Rust semantics.
Validation on this revision: 170 local Python tests passed, 82 skipped, 1 deselected, including all 26 executed counting parity checks; 117 Rust unit tests passed, 7 ignored. Ruff source/tests, formatting and diff checks passed. GitHub checks are running on the new head.
Limitations: per-SNP fetching may increase index seeks and repeated decoding; large-cohort performance has not been measured. Analysis references are synthetic. The archived linear model requires explicit scalar/array adapters and ignores the phased argument for multi-SNP tests; those four cases are not certified as parity. This does not establish complete donor statistical parity. PR #133 depends on this compatibility change. No merge or release is included.