search

owkin / PyDESeq2

A Python implementation of the DESeq2 pipeline for bulk RNA-seq DEA.
https://pydeseq2.readthedocs.io/en/latest/
MIT License
584 stars 62 forks source link

support similar apeAdapt argument as in DESeq2::lfcShrink() #264

Closed awalsh17 closed 6 months ago

awalsh17 commented 7 months ago

Is your feature request related to a problem? Please describe. This is not a problem, but a limitation for some datasets. In the R package, the DESeq2::lfcShrink() wrapper that calls {apeglm} includes the apeAdapt argument that, when set to FALSE, does not use the logFC MLE and sets mle = NULL for apeglm() call. This thread explains some of the reasoning to do this: https://support.bioconductor.org/p/111544/

Describe the solution you'd like Would you support adding an argument to lfc_shrink that would perform similarly to the R implementation? If you pointed me in the right direction in the code, I could implement it. If it is as simple as changing prior_scale to 1, then this might be simple. (Based on my read of the code here: https://github.com/azhu513/apeglm/blob/master/R/apeglm.R)

Describe alternatives you've considered Just use the R package

Additional context Thank you!

BorisMuzellec commented 7 months ago

Hi @awalsh17, thanks for your interest in pydeseq2! I'd be happy to help you open a PR to implement this option.

After a brief look at the code I understand the same as you, which is that we should just add an option in lfc_shrink to set prior_scale=1. If you'd like to open a PR, I'll add some test cases to check that our understanding is correct.

Thanks!

awalsh17 commented 7 months ago

Thanks @BorisMuzellec ! I will submit a PR with the proposed change. I tested something locally with my test data, and it "works," but I should also compare it to the R implementation.