-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add gaussian_mixture_model_karyotype_assignment
function to assign sex karyotype using Gaussian mixture models
#478
Conversation
If helpful for testing:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good to me. Mostly just small suggestions on docs and a couple errors to raise. However, I hit a bug in the old code when testing your example 2 and 4 above, that I cannot comment on in the PR but hit when testing your example 2 and 4 above -- using a MT in the annotate_sex function. The call to impute_sex_ploidy returns a {contig}_mean_dp
field and not an autosomal_mean_dp
gnomad_methods/gnomad/utils/sparse_mt.py
Line 672 in 7a64393
f"{chrom}_mean_dp": hl.agg.filter( |
gnomad_methods/gnomad/utils/sparse_mt.py
Line 682 in 7a64393
f"{chrom}_mean_dp": hl.agg.sum( |
autosomal_mean_dp
if variants only is true but that field doesnt exist, and if it is false, it attempts to rename to {contig}_mean_dp
which already exists. Unless there is a subsequent PR that will update this and it was accidentally split apart from this PR, well need to fix it. Do you want me to make a ticket for this or should this PR cover it?
Co-authored-by: Mike Wilson <mwilson@broadinstitute.org>
…nstitute/gnomad_methods into jg/add_gaussian_karyotype � Conflicts: � gnomad/sample_qc/pipeline.py
Yes, the bug you found does get fixed in the next PR, but I might as well fix it here too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Co-authored-by: Mike Wilson <mwilson@broadinstitute.org>
Modify
get_ploidy_cutoffs
to accept an expression for grouping samples into 'XX' and 'XY' groupings.Modify
annotate_sex
to allow for sex karyotype imputation to use a gaussian mixture model to determine grouping of samples into 'XX' and 'XY' for use inget_ploidy_cutoffs
.Modify
annotate_sex
to make computing f-stat and inferring sex karyotype optional.