Skip to content
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

Merged
merged 40 commits into from
Oct 16, 2023

Conversation

jkgoodrich
Copy link
Contributor

No description provided.

…to jg/combined_faf

# Conflicts:
#	gnomad_qc/v4/resources/annotations.py
Copy link
Contributor

@mike-w-wilson mike-w-wilson left a 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.

@jkgoodrich jkgoodrich marked this pull request as ready for review October 13, 2023 08:13
Copy link
Contributor

@mike-w-wilson mike-w-wilson left a 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(
Copy link
Contributor

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.

Copy link
Contributor

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)

Copy link
Contributor

@mike-w-wilson mike-w-wilson left a 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!

Copy link
Contributor

@mike-w-wilson mike-w-wilson left a comment

Choose a reason for hiding this comment

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

:shipit:

@jkgoodrich jkgoodrich merged commit 243f21f into main Oct 16, 2023
1 check passed
@jkgoodrich jkgoodrich deleted the jg/combined_faf branch October 16, 2023 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants