Repository navigation
Phased attribute to HaplotypeMatrix and warnings in phase-dependent statistics. - #401
Merged
Merged
Conversation
mufernando
force-pushed
the
phase-stats
branch
from
November 6, 2026 23:46
819330f to
cca030a
Compare
andrewkern
reviewed
Nov 6, 2026
andrewkern
left a comment
Member
There was a problem hiding this comment.
This looks great to me. A couple findings:
- Needs a rebase:
windowed_analysis.pyconflicts with main in two hunks (keep both sides). After rebasing,pixi run check-mapfails on 6 map entries: the newphasedparam onHaplotypeMatrix.__init__,from_vcf,from_zarr,from_ts, andphased/set_phased/set_unphasedon both haplotype matrix classes indocs/dev/pipeline_map.toml. - The
pyproject.tomledit changes the hash of the editablepg_gpuentry inpixi.lock, so CI's locked install will fail until you runpixi lockand commit the lock. - Lint:
tests/test_unphased_warning.pyhas two E402s and an E731. - Since we require Python 3.12,
_skip_phase_wrapperscan be replaced bywarnings.warn(..., skip_file_prefixes=(os.path.dirname(pg_gpu.__file__) + os.sep,)). I checked it gives the same warning locations on 11 paths, and it removes the frame walk and the hard-coded stacklevels (the ContextVar depth counter is still needed). to_zarrthenfrom_zarrdrops the flag, so afrom_tsmatrix saved to zarr comes back unphased. Writing and reading VCZ'scall_genotype_phasedwould keep it; fine as a follow-up.
Member
Author
|
I addressed your concerns, @andrewkern! I'll let you squash when merging. |
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.
This addresses #168.
Haplotype_Matrix, which is the main pg-gpu data structure, did not have a notion of phase. This is critical because a lot of statistics can only be run with phased data, whereas others work either way. This is already dealt with at some level, e.g. withpcavspca_dosage, but I think it's important for users to be warned if they try to compute phase-dependent statistics on unphased data.The changelog summarizes all the changes well, I think:
Added
phasedattribute toHaplotypeMatrix(which defaults toFalse). This tracks whether the haplotypes in the matrixrepresent true phased haplotypes or not. Each loader exposes a
phasedkeyword argument:from_vcfandfrom_zarrdefaultto
False;from_tsdefaults toTrue. It is possible tocall
set_phased()orset_unphased()to change it afterconstruction.
Statistics that depend on phased haplotypes emit an
UnphasedDataWarningwhen given aHaplotypeMatrixwithphased=False. These include Garud's H, iHS, XP-EHH, nSL,haplotype counts and diversity, haplotype r² and D, distance
statistics between haplotypes, PCA and local PCA on haplotypes, and the
windowed scans that request any of these. Each call warns once and it
names an unphased alternative where one exists. For
garud_h,pairwise_diffs, thedist_*moments,znsandomega, the unphased alternative is to pass aGenotypeMatrix.One small wrinkle:
moments_ld.compute_ld_statisticswithvcf_file=anduse_genotypes=Falsenow loads the VCF as phased,just to avoid the unphased data warning. See :doc:
phasingfor the fulllist of statistics and how to silence the warning.
Because
from_vcf,from_zarr, the rawHaplotypeMatrix(...)constructor, and
GenotypeMatrix.to_haplotype_matrixall givephased=False, going forward all code that uses phased data withoutexplicitly marking it as phased will start to see this warning. Load
with
phased=Trueor callset_phased()after loading.