-
Notifications
You must be signed in to change notification settings - Fork 43
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
Release browser hail tables #1583
Conversation
06c96f5
to
0dcf09f
Compare
36f6674
to
87e6c78
Compare
Files to sync 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.
Hey this looks good. I flagged what looks like a few typos.
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.
thank you for adding! I've added some minor comments and suggestions, mostly to help clarify the descriptions
Thanks, all, for the comments. Going to be working through them right now. Thanks especially to KC for your patience, and for going through this whole blurb of text multiple times now. |
ea308ca
to
68481d7
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.
a few more minor things -- thanks for your patience with my nitpicks!
e202f3f
to
c2f4135
Compare
c2f4135
to
4a6d0e2
Compare
Hiya @ch-kr, this should be ready for your re-review, at your convenience. Apart from addressing the comments there's a few added things (downloads page with placeholder links, the changelog post , and documenting of the VRS annotations and the joint frequency annotation). Thank you!! |
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.
thank you for adding the downloads! a few more minor things, otherwise LGTM. the one thing that needs updating is in data-pipeline/src/data_pipeline/data_types/gene.py
(it looks like a bug to me)
data-pipeline/src/data_pipeline/pipelines/gnomad_v4_variants.py
Outdated
Show resolved
Hide resolved
7ff320c
to
26d5734
Compare
26d5734
to
004c404
Compare
afd0d3a
to
0f1b1a9
Compare
0f1b1a9
to
c232c45
Compare
Resolves #1369
Adds a help page (à la the v4 hail table help page) that describes the gnomAD browser hail tables (variants, gene models).
Reviewing this PR mostly entails taking a glance over the help text to see if there's anything incorrect or misleading.