owkin / PyDESeq2

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

Add adapt = False (prior_scale=1) option to lfc_shrink() #267

Closed awalsh17 closed 5 months ago

awalsh17 commented 5 months ago

Reference Issue or PRs

This is in reference to #264

What does your PR implement? Be specific.

This adds an option to lfc_shrink() to allow one to keep prior_scale = 1. This is equivalent to the R package implementation of DESeq2::lfcShrink( apeAdapt = FALSE )

I tested some and the behavior is similar to the R package. But not 100%. I am not sure, but likely a difference in the parameters I passed to each. These plots are from using the same data, contrast, and adapt = False argument.

image

BorisMuzellec commented 5 months ago

Thanks a lot for this PR @awalsh17! I'll try to add a test to automatically compare with deseq2 by the end of the week.

BorisMuzellec commented 5 months ago

I tested some and the behavior is similar to the R package. But not 100%. I am not sure, but likely a difference in the parameters I passed to each. These plots are from using the same data, contrast, and adapt = False argument.

@awalsh17 in those plots did you run lfc shrinkage starting from the same size factors, dispersions and LFCs, or did you run a DESeq2 pipeline and a PyDESeq2 pipeline independently (starting from the same data)?

awalsh17 commented 5 months ago

I tested some and the behavior is similar to the R package. But not 100%. I am not sure, but likely a difference in the parameters I passed to each. These plots are from using the same data, contrast, and adapt = False argument.

@awalsh17 in those plots did you run lfc shrinkage starting from the same size factors, dispersions and LFCs, or did you run a DESeq2 pipeline and a PyDESeq2 pipeline independently (starting from the same data)?

@BorisMuzellec Good point. I ran them completely independently (starting from the same data). I could repeat using the same data input to just the lfc shrinkage?

BorisMuzellec commented 5 months ago

@BorisMuzellec Good point. I ran them completely independently (starting from the same data). I could repeat using the same data input to just the lfc shrinkage?

If you have time for it, a good sanity check would be to test on real data indeed, but for this we would need to factor out differences between DESeq2 and PyDESeq2 in terms of dispersions by manually setting the same dispersions in both packages.

Otherwise, I think that the PR passing tests on the synthetic test data is good enough