Skip to content

adding phased property to HaplotypeMatrix - #354

Closed
mufernando wants to merge 2 commits into
kr-colab:mainfrom
mufernando:add-phase
Closed

mufernando wants to merge 2 commits into
kr-colab:mainfrom
mufernando:add-phase

Conversation

@mufernando

Copy link
Copy Markdown
Member

WIP

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The streaming constructor breaks existing callers by requiring a new argument, and the quickstart contains inaccuracies about VCF ploidy, haplotype values, and dosage conversion.

4 open issues
What changed in this PR

Adds informational phasing metadata to eager and streaming haplotype matrices, preparing the codebase for future phase-aware warnings without changing statistical results.

Changes:

  • Adds phased and set_phased(), with loader-specific defaults.
  • Preserves phasing through subsets, filtering, streaming chunks, and windowed matrices.
  • Adds tests, updates documentation, and outlines future warning behavior.
File Description
tests/​test_streaming.py Supplies phasing explicitly in streaming construction.
tests/​test_phasing.py Tests defaults, overrides, propagation, and conversions.
tests/​test_gpu_genotype_prep.py Updates builder calls with explicit phasing.
PHASING_PLAN.md Outlines future phase-dependent warnings and follow-ups.
pg_gpu/​windowed_analysis.py Preserves phasing in per-window matrices.
pg_gpu/​streaming_matrix.py Adds streaming phasing metadata and chunk propagation.
pg_gpu/​haplotype_matrix.py Adds the flag, loader defaults, and propagation.
pg_gpu/​genotype_matrix.py Marks reconstructed haplotypes as unphased.
pg_gpu/​_utils.py Preserves phasing in population subsets.
pg_gpu/​_gpu_genotype_prep.py Requires and forwards explicit phasing.
docs/​source/​quickstart.rst Explains phasing flags and matrix conversions.
docs/​source/​changelog.rst Announces informational phasing support.
docs/​source/​api.rst Exposes streaming phasing members in API documentation.

Review effort: Balanced

"""

def __init__(self, source, fetcher, chunk_bp, prefetch, *,
phased, accessible_mask=None, n_total_sites=None):
Comment thread docs/source/quickstart.rst Outdated
Comment on lines +11 to +12
per haplotype, with variants as columns). When loaded from a diploid
(or n-ploid) VCF, the two haploid copies of each sample become 2 (or n) rows.
Comment thread docs/source/quickstart.rst Outdated
Comment on lines +542 to +543
``HaplotypeMatrix`` rows are gametes (0/1 per allele, two rows per
individual). ``GenotypeMatrix`` rows are individuals (0/1/2 dosage).
Comment thread docs/source/quickstart.rst Outdated
Comment on lines +551 to +553
# Convert from haploid (pairs consecutive haplotypes).
# The result is always phased=False: heterozygotes are assigned as
# (0, 1), which is a pseudo-phasing.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new required phased argument breaks existing callers of the public streaming constructor, and the quickstart contains inaccurate loading and conversion guidance.

5 open issues

Review effort: Balanced

Comment on lines +546 to +548
(0/1/2 dosage). Use ``GenotypeMatrix`` only for statistics which need
dosage encoding. Many functions auto-dispatch on input type, so the same
call works on either a ``HaplotypeMatrix`` or a ``GenotypeMatrix``.
@mufernando mufernando closed this Oct 30, 2026
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.

2 participants