Skip to content

Conversation

@mike-w-wilson
Copy link
Contributor

@mike-w-wilson mike-w-wilson commented Jan 9, 2024

The genetic ancestry group Middle Eastern should be considered in grpmax and faf calculations for v4. This was not considered in v3 so we're splitting these globals into version specific dictionaries where the keys are gnomad versions and values are the genetic ancestry groups. This follows the behavior of POPS.

Copy link
Contributor

@klaricch klaricch left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +225 to +226
"v3": {"asj", "fin", "mid", "oth", "ami", "remaining"},
"v4": {"asj", "fin", "oth", "ami", "remaining"},
Copy link
Contributor

Choose a reason for hiding this comment

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

either way this is fine, just wondering does v3 have "remaining" and does v4 have "oth"? i don't remember what steps the name changes went in

Copy link
Contributor Author

@mike-w-wilson mike-w-wilson Jan 24, 2024

Choose a reason for hiding this comment

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

Yeah this is a bit annoying, v4 genomes uses remaining but v3 genomes used oth but the code needs both for backwards compatibility and since the v4 genomes uses the v3 set because we still have fewer than 1000 mid samples in the v4 genomes....this is why we need a resources overhaul.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept oth in v4 as a safety measure here because of the joint_frequency work, thats also why ami is in v4 even though v4 exomes do not have ami samples

@mike-w-wilson mike-w-wilson merged commit 89548e7 into main Jan 24, 2024
@mike-w-wilson mike-w-wilson deleted the mw/mid_in_faf branch January 24, 2024 13:57
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.

2 participants