search

cumc / xqtl-protocol

Molecular QTL analysis protocol developed by ADSP Functional Genomics Consortium
https://cumc.github.io/xqtl-protocol/
MIT License
33 stars 42 forks source link

update coloc analysis - sos loop by context #918

Closed rfeng2023 closed 4 months ago

rfeng2023 commented 4 months ago

Hi @gaow please review it before you merge it. Especially for Analytical Logic part. In this strategy, we only focus on the genes which have top loci results, and GWAS blocks that contain top loci variants overlapping with QTL.

review-notebook-app[bot] commented 4 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

gaow commented 4 months ago

@rfeng2023 will do tomorrow but if i forget please remind me again.

gaow commented 4 months ago

@rfeng2023 the rough logic looks good although the interface and codes needs clean up. However I think there is a misunderstanding about enrichment analysis. Currently simply taking overlap I think correspond to a wrong statistical model. The fix is straightforward but conceptually we need to clarify it. Let's talk more in person.

gaow commented 4 months ago

@rfeng2023 I added a check and an adjustment to ensure that variant names match between GWAS and xQTL for enloc. Otherwise it's not going to work.

https://github.com/cumc/pecotmr/blob/main/R/compute_qtl_enrichment.R#L91

I had to define this funciton:

https://github.com/cumc/pecotmr/blob/main/R/allele_qc.R#L212

See usage here in all these unit tests i added:

https://github.com/cumc/pecotmr/blob/main/tests/testthat/test_allele_qc.R#L144

Please test in real data. let me know if you find any issues or logic errors not coding errors in this function. Please also use this function in your result export codes to seriously address this. The codes you currently has in the export codes are too simplified.

gaow commented 4 months ago

@rfeng2023 thanks -- you think it is ready for another round of comment?

rfeng2023 commented 4 months ago

@rfeng2023 thanks -- you think it is ready for another round of comment?

@gaow Yes this version is modified based on we discussed this morning, please review it. Thanks!

gaow commented 4 months ago

In pecotmr, https://github.com/cumc/pecotmr/blob/main/R/encoloc.R#L120C1-L120C88 analysis_script_obj this is not necessary? if so please remove.

gaow commented 4 months ago

Also a very minor issue: there is a print statement in the workflow persumably for debug purpose at one time. Please remove it now.