Copy of: Add gnomad_gks() and get_gks() for extracting gks information for a specified variant #596

matren395 · 2023-09-11T00:33:46Z

Copy of Pull Request "Add gnomad_gks() and get_gks() for extracting gks information for a specified variant" with only relevant changes included. The Git History got extremely confused when consulting with Steve Jahl, and his suggestion in the face of the problem was to copy the relevant changes to a new PR/branch, given how complicated to resolve the GitHistory would have been

theferrit32 · 2023-09-19T18:56:48Z

gnomad/resources/grch38/gnomad.py

+    # Select relevant fields, checkpoint, and filter to interval before adding
+    # annotations
+    ht = ht.select(ht.freq, ht.info.vrs, ht.popmax)
+    ht = ht.checkpoint(hl.utils.new_temp_file("vrs_checkpoint", extension="ht"))


If I can make one last suggestion, could there be an optional argument like disable_checkpoint to gnomad_gks that turns off this checkpoint, and a doc string that says it is recommended to leave on for larger datasets?

I'm just observing that for small tables that are more like tens of thousands of variants and <1GB in size and can comfortably fit in memory, the checkpoint adds a lot of time overhead.

Thats definitely possible. We are also about to make one more change for Popmax (where AF =/= FAF) so yours isn't the last suggestion:)

Awesome, thanks @matren395

@matren395 I can push a branch with that and create another multi-tiered PR

klaricch · 2023-09-27T13:13:54Z

gnomad/resources/grch38/gnomad.py

    # Select relevant fields, checkpoint, and filter to interval before adding
    # annotations
-    keep_fields = [ht.freq, ht.info.vrs, ht.popmax]
+    keep_fields = [ht.freq, ht.info.vrs, ht.popmax, ht.faf]


drop ht.pomax (we're pulling faf instead)

you could add the code for defining the popmaxFAF (similar to the browser code) above keep_fields, select faf95 here, then within add_gks_va just grab faf95.popmax and faf95. popmax_population if faf95 exists in the struct

okay that's a lot smarter, thank you!

gnomad/resources/grch38/gnomad.py

klaricch · 2023-09-27T13:52:29Z

gnomad/utils/annotations.py

+            "frequency": faf95_dict[-1],
            "confidenceInterval": 0.95,
-            "popFreqId": f"{gnomad_id}.{input_struct.popmax.pop.upper()}",
+            "popFreqId": f"{gnomad_id}.{faf95_dict.pop.upper()}",


where did faf95_dict get the pop struct?

klaricch · 2023-09-27T13:55:01Z

gnomad/utils/annotations.py

+
+    faf95_dict = sorted(sorted_faf95, key=lambda x: x[1])[-1]
+
+    if faf95_dict[-1] > 0.0:


in the browser it looks like if popmaxfaf is 0, it is still displayed, it just doesn't have a hover over, so we should probabaly copy that same behavior

returns a 0 value now and only gives None for popFreqID

gnomad/utils/annotations.py

klaricch · 2023-09-27T16:34:41Z

gnomad/resources/grch38/gnomad.py

+from typing import Optional, Union
+
+import hail as hl
+from gnomad_qc.v3.create_release.prepare_vcf_data_release import FAF_POPS


to maintain proper isort order, move imports from "gnomad_qc" below ones from "gnomad"

that's odd, I ran isort on it? maybe running black afterwards messed it back up - i'll address it

klaricch · 2023-09-27T16:35:26Z

gnomad/resources/grch38/gnomad.py

 )


+def get_coverage_ht(


delete this function

klaricch · 2023-09-27T16:38:19Z

gnomad/utils/annotations.py

    new_freq = freq_meta_idx.map(
        lambda x: hl.bind(
-            lambda y: y.annotate(AF=hl.or_missing(y.AN > 0, y.AC / y.AN)).select(
+            lambda y: y.annotate(AF=hl.if_else(y.AN > 0, y.AC / y.AN, 0)).select(


what's this change for?

I don't recall touching this manually, but I looked back and this change occurred when I was copying in the code from the old branch and setting up the PR. Apparently something was either ahead or behind of something else.

but anyways I reverted it back to the or_missing() functionality

matren395 · 2023-09-27T17:36:32Z

rebased with Steve's method!

theferrit32 · 2023-09-28T16:20:06Z

requirements.txt

@@ -1,4 +1,5 @@
 annoy
+ga4gh.vrs[extras]


I think the version here should probably be pinned so when we release a vrs-python non-alpha/beta on a 1.x or 2.x tag this doesn't break. The version discussed previously was 0.8.4 but assuming vrs-python sticks by semantic versioning it could be something wider.

requirements.txt

change name of 'input_dict' to 'input_struct' to reflect the fact that it is an input Hail Struct (even though it can be indexed like a Python dictionary) - and a few capitalization changes in there as well

may have some more renaming to be done or to run Black from command line, tbd Co-authored-by: klaricch <[email protected]>

with freq_index_dict and sex_id fixes

select and checkpoint added

run Black and upgrade example interval to a gene I was testing with - ESPN gene in Chr1 - that doesn't error out at the collect statement

Black's method for reformatting that inline comment is really awkward , huh

thanks for the fix! Co-authored-by: klaricch <[email protected]>

…rage_ht

Revise Faf95 work , but this implementation is probably not perfect yet!

matren395 self-assigned this Sep 11, 2023

matren395 mentioned this pull request Sep 12, 2023

Add gnomad_gks() and get_gks() for extracting gks information for a specified variant #539

Closed

matren395 added Changelog: new feature ga4gh VRS labels Sep 12, 2023

matren395 requested review from klaricch, jkgoodrich and ch-kr September 12, 2023 14:36

klaricch self-assigned this Sep 18, 2023

theferrit32 reviewed Sep 19, 2023

View reviewed changes

theferrit32 mentioned this pull request Sep 20, 2023

Optimize a couple collect calls and add option to disable checkpointing #601

Merged

klaricch requested changes Sep 27, 2023

View reviewed changes

klaricch self-requested a review September 27, 2023 16:31

klaricch requested changes Sep 27, 2023

View reviewed changes

matren395 requested a review from klaricch September 27, 2023 17:27

matren395 force-pushed the dm/variant_report_code_rebranch branch from 463653d to b315b68 Compare September 27, 2023 17:36

klaricch approved these changes Sep 27, 2023

View reviewed changes

matren395 requested a review from theferrit32 September 27, 2023 18:30

theferrit32 suggested changes Sep 28, 2023

View reviewed changes

matren395 requested a review from klaricch September 28, 2023 19:03

matren395 commented Sep 28, 2023

View reviewed changes

requirements.txt Outdated Show resolved Hide resolved

theferrit32 approved these changes Sep 28, 2023

View reviewed changes

matren395 and others added 8 commits September 28, 2023 15:14

Copy relevant changes from variant_report_code

b1d89cf

Copy relevant changes to branch

f3b8864

Run Black on gnomad.py

0b3321f

isort

96cdd95

DocString and one variable name

4533e20

change name of 'input_dict' to 'input_struct' to reflect the fact that it is an input Hail Struct (even though it can be indexed like a Python dictionary) - and a few capitalization changes in there as well

Update variable name in gnomad.py

120e906

Apply suggestions from code review

aed049a

may have some more renaming to be done or to run Black from command line, tbd Co-authored-by: klaricch <[email protected]>

Update annotations.py

381f233

with freq_index_dict and sex_id fixes

matren395 and others added 19 commits September 28, 2023 15:14

add select and checkpoint

769265f

select and checkpoint added

Update gnomad.py

2309908

run Black and upgrade example interval to a gene I was testing with - ESPN gene in Chr1 - that doesn't error out at the collect statement

Black & Renaming

28833bc

Black's method for reformatting that inline comment is really awkward , huh

revise checkpoint structure

46d0c70

new_temp_file not _path oops:)

be68c7a

Update gnomad/resources/grch38/gnomad.py

6c97aeb

thanks for the fix! Co-authored-by: klaricch <[email protected]>

PyLint!!!

0a12289

Only collect globals once

1332bb8

Perform coverage join in gnomad_gks so there's only 1 collect on cove…

51868fc

…rage_ht

Add skip_checkpoint argument to gnomad_gks

6c686b5

Apply review comments.

c0cf68d

Fix field name

0d465a9

revise popmax

1f95734

Revise Faf95 work , but this implementation is probably not perfect yet!

Move FAF95 annotation and edit some handling

728620e

revise import, remove weird change, remove old fxn

c5bffa2

reformat gnomad.py with Black

120c25c

rerun isort==5.12.0

6a35362

simpler FAF_POPS import

263346a

Update requirements.txt

1d1e517

matren395 force-pushed the dm/variant_report_code_rebranch branch from d0ac270 to 1d1e517 Compare September 28, 2023 19:14

matren395 merged commit f8e4313 into main Sep 28, 2023


		faf95_dict = sorted(sorted_faf95, key=lambda x: x[1])[-1]

		if faf95_dict[-1] > 0.0:

		)


		def get_coverage_ht(

Copy of: Add gnomad_gks() and get_gks() for extracting gks information for a specified variant #596

Copy of: Add gnomad_gks() and get_gks() for extracting gks information for a specified variant #596

Uh oh!

Conversation

matren395 commented Sep 11, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

klaricch Sep 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matren395 commented Sep 27, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

klaricch Sep 27, 2023 •

edited

Loading