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

feat: LD index and block matrix extraction for a studyLocus #463

Merged
merged 47 commits into from
Apr 3, 2024

Conversation

Daniel-Considine
Copy link
Contributor

Adds get_gnomad_matrix() function to study_locus.py. This function takes a study locus, finds the relevant study ID, extracts the major ancestry, and then returns the ancestry specific hail ld index and block matrices for that window.

@codecov-commenter
Copy link

codecov-commenter commented Feb 2, 2024

Codecov Report

Attention: 175 lines in your changes are missing coverage. Please review.

Comparison is base (42b366c) 85.67% compared to head (99fbf48) 85.94%.
Report is 115 commits behind head on dev.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #463      +/-   ##
==========================================
+ Coverage   85.67%   85.94%   +0.27%     
==========================================
  Files          89      100      +11     
  Lines        2101     2875     +774     
==========================================
+ Hits         1800     2471     +671     
- Misses        301      404     +103     
Files Coverage Δ
src/airflow/dags/common_airflow.py 90.38% <100.00%> (ø)
src/airflow/dags/dag_preprocess.py 100.00% <ø> (ø)
src/airflow/dags/finngen_preprocess.py 100.00% <100.00%> (ø)
src/airflow/dags/gwas_curation_update.py 100.00% <100.00%> (ø)
src/gentropy/__init__.py 100.00% <ø> (ø)
src/gentropy/assets/__init__.py 100.00% <ø> (ø)
src/gentropy/assets/data/__init__.py 100.00% <ø> (ø)
src/gentropy/assets/schemas/__init__.py 100.00% <ø> (ø)
src/gentropy/cli.py 91.66% <100.00%> (ø)
src/gentropy/common/Liftover.py 80.64% <ø> (ø)
... and 71 more

... and 38 files with indirect coverage changes

@addramir
Copy link
Contributor

addramir commented Feb 5, 2024

Is it ready for the review?

@Daniel-Considine Daniel-Considine marked this pull request as ready for review February 5, 2024 13:49
@Daniel-Considine
Copy link
Contributor Author

I haven't yet added tests for the functions I have added to study_locus.py

@DSuveges
To get the tests to pass on some of the pairwise_ld.py code I've just hacked it together to get it working, so there is probably some issues here. Do you think the functions I've written would make more sense to be a part of pairwise_ld.py instead?

@DSuveges
Copy link
Contributor

DSuveges commented Feb 9, 2024

To extract the square NumPy matrix for a random study locus (5_59546948_C_T) using a 1 mega-base window in non-finish European populations takes about 40s running on my laptop.

Nice. I'll review this PR together with the Z-score one today.

Copy link
Contributor

@DSuveges DSuveges left a comment

Choose a reason for hiding this comment

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

My only concern is the layout of the locus index retriever function. Which functionality is more or less already in this function: get_ld_variants here

src/gentropy/datasource/gnomad/ld.py Outdated Show resolved Hide resolved
src/gentropy/datasource/gnomad/ld.py Outdated Show resolved Hide resolved
@Daniel-Considine Daniel-Considine marked this pull request as draft March 28, 2024 14:15
@Daniel-Considine Daniel-Considine marked this pull request as ready for review April 2, 2024 15:46
Copy link
Contributor

@DSuveges DSuveges left a comment

Choose a reason for hiding this comment

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

OK, I think all makes sense now. In a separate effort we can update the other function that the ld index generation uses.

@DSuveges DSuveges merged commit 56067e7 into dev Apr 3, 2024
4 checks passed
@Daniel-Considine Daniel-Considine deleted the ds_pairwise_ld branch April 3, 2024 14:30
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.

4 participants