-
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
Minor GKS formatting changes and addition of gnomAD flags to annotation #617
Minor GKS formatting changes and addition of gnomAD flags to annotation #617
Conversation
84a0f3b
to
373c0f2
Compare
Hey Kyle, could we roll the change to "popFreqId" not returning "None" but instead a string here as well? I had created my own PR for that and the "popFreqID" fix, but it looks like this already contains the latter and would be easy enough to host the former as well |
36d3d24
to
011c298
Compare
One more pending change is to add @larrybabb @ahwagner Was there an idea to move some of these fields out of see: ga4gh/va-spec#119 (comment) I think some of that might have been related to my confusion. I'm not sure the allele balance flagged actually are directly related to the popmax FAF 95. Maybe someone on the gnomad team can clarify. |
commit to roll in 1) nullable popMaxFAF95 and 2) change all references of 'pop' returned to 'grp' when relevant. |
Waiting for final modifications to upstream schema. There is one more discussion tomorrow. The changes will be relatively easy to implement after that. |
Adding commits and force pushing a branch rebased from main |
4a77e59
to
e1b8b96
Compare
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.
might take another run at the actual code in annotations.py later today
Latest changeset: https://github.com/theferrit32/gnomad_methods/compare/374112031d6b1178e40cfc23161a19b82b9d5693..74d869e022933612e566755f42d8dd2e0445e114
|
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.
notes! lmk if you wanna chat abt the PR next week
coverage("exomes").versions[coverage_version].path | ||
) | ||
coverage_ht = hl.read_table( | ||
coverage(data_type).versions[coverage_version].path |
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.
@klaricch this will use genomes
version 3.0.1
coverage table for v3 and v4 genomes tables, and exomes
version 4.0
coverage table for v4 exomes tables. If the input is a v3 table with data_type=exomes
, line 562 will throw an exception KeyError: '3.0.1'
, which is expected.
Rebasing from main to fix lint error from duplicate |
99bad94
to
537a1d9
Compare
…e_group_dicts, refactor to remove idx as param, construct ids and get freq idx locally
…r and allele balance structures in v3/v4
…axFAF95 if null to conform with latest schema. Move meanDepth to qualityMeasures
Co-authored-by: Daniel Marten <78616802+matren395@users.noreply.github.com> Co-authored-by: klaricch <kristen@broadinstitute.org>
Co-authored-by: klaricch <kristen@broadinstitute.org>
be25fdb
to
caabf24
Compare
Modifications:
popFreqID
topopFreqId
.
delimiter instead of,
to match the popmax syntaxAdditions:
heterozygousSkewedAlleleCount
One point is that the allele balance code will fail if the number of entries or the bins in the array
ht.qual_hists.ab_hist_alt.bin_freq
changes. That field is directly used to drive the the UI.EDIT:
Additional changes:
popmaxFAF95
togrpMaxFAF95
,popFreqId
togrpFreqId
(@matren395)