-
Notifications
You must be signed in to change notification settings - Fork 26
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
Script to compute combined FAF of v4 genomes and exomes #403
Conversation
…to jg/combined_faf # Conflicts: # gnomad_qc/v4/resources/annotations.py
…to jg/combined_faf
…nomad_qc into qh/combined_faf
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.
I've reviewed this one first -- it's possible some things are address in @KoalaQin 's but since thats merging into this branch, I only see some diffs. I'll review that one next.
Script to compute combined FAF of v4 genomes and exomes
…to jg/combined_faf
…to jg/combined_faf
…to jg/combined_faf # Conflicts: # gnomad_qc/v4/resources/annotations.py
Add code for creating the final combined FAF release HT
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.
This looks good, just a couple questions and suggestions!
ht = ht.annotate( | ||
joint_freq=freq, | ||
joint_faf=faf, | ||
joint_fafmax=gen_anc_faf_max_expr( |
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.
General question -- maybe for @ch-kr , should this be put on the release? I know you were looking into this a bit more so curious what your thoughts are.
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.
from chatting with Anne, the FAFs that are actually used by analysts are the values included in this struct (=values that correspond to FAFs from gen anc group with highest FAF for variant, not FAFs from gen anc group with highest AF group for variant); see #477 (comment)
Co-authored-by: Mike Wilson <[email protected]>
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.
Just one thing and then this looks good!
Co-authored-by: Mike Wilson <[email protected]>
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.
No description provided.