-
Notifications
You must be signed in to change notification settings - Fork 23
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
Switch to formulaic-contrasts #682
Conversation
@grst I'll fix the unrelated tests soon. Sorry about them + the currently uber slow CI. |
What's left is to update the differential expression tutorial to use a numeric contrast vector instead of a tuple of strings, i.e. -res_df = pds2.test_contrasts(["Treatment", "Chemo", "Anti-PD-L1+Chemo"])
+res_df = pds2.test_contrasts(pds2.contrast(column="Treatment", baseline="Chemo", group_to_compare="Anti-PD-L1+Chemo")) I tried a bit, but rerunning the tutorial with edgeR and rpy2 is a huge rabbit hole. |
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.
LGTM. I've pushed some edits to pass the rpy2/edgeR tests #692
The main thing to work on seems to be the documentation, e.g. is this still accurate?
pertpy/pertpy/tools/_differential_gene_expression/_base.py
Lines 982 to 992 in 298db0f
def test_contrasts(self, contrasts, **kwargs): | |
""" | |
Perform a comparison as specified in a contrast vector. | |
Args: | |
contrasts: Either a numeric contrast vector, or a dictionary of numeric contrast vectors. | |
**kwargs: passed to the respective implementation. | |
Returns: | |
A dataframe with the results. | |
""" |
Also typing in base functions like test_contrasts
and compare_groups
would be helpful.
I'm gonna have a go at editing the tutorial. |
One small note: I noticed that the model doesn't complain if you try to specify a complex interaction contrast on a model that wasn't fit with the interaction in the design, but just throws nonsense results. Following example from the tutorial: # Exclude patient with progressive disease, or not full rank for interaction
pdata2 = pdata[pdata.obs['Efficacy'] != 'PD'].copy()
# Bad design definition without interaction
pds2 = pt.tl.PyDESeq2(adata=pdata2, design="~ Efficacy + Treatment")
pds2.fit()
interaction_contrast = (
pds2.cond(Treatment="Chemo", Efficacy="PR") - pds2.cond(Treatment="Chemo", Efficacy="SD")
) - (
pds2.cond(Treatment="Anti-PD-L1+Chemo", Efficacy="PR") - pds2.cond(Treatment="Anti-PD-L1+Chemo", Efficacy="SD")
)
res_df = pds2.test_contrasts(contrasts=interaction_contrast) No complaint, but the results are broken: Log2 fold change & Wald test p-value, contrast vector: [0. 0. 0.]
baseMean log2FoldChange lfcSE stat pvalue padj
A1BG 16.408605 0.0 0.0 NaN NaN NaN
A1BG-AS1 1.958737 0.0 0.0 NaN NaN NaN
A1CF 0.002053 0.0 0.0 NaN NaN NaN
A2M 30.296881 0.0 0.0 NaN NaN NaN
A2M-AS1 0.557092 0.0 0.0 NaN NaN NaN
... ... ... ... ... ... ...
ZXDC 6.114098 0.0 0.0 NaN NaN NaN
ZYG11A 0.093600 0.0 0.0 NaN NaN NaN
ZYG11B 3.404941 0.0 0.0 NaN NaN NaN
ZYX 77.175203 0.0 0.0 NaN NaN NaN
ZZEF1 9.752162 0.0 0.0 NaN NaN NaN While results make sense if I specify the design properly Do we want this to happen? Can we add an informative error if the contrast vector is all zeros? |
* fix broken rpy2 edger tests * updated edger tests
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
Signed-off-by: zethson <[email protected]>
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.
<3 looks great!
Signed-off-by: zethson <[email protected]>
@emdann I added type hints as you requested above and updated the submodule to have your tutorial changes as well. With passing CI (I work on it), I am happy to merge this now. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #682 +/- ##
==========================================
- Coverage 65.56% 64.85% -0.72%
==========================================
Files 47 46 -1
Lines 6105 5992 -113
==========================================
- Hits 4003 3886 -117
- Misses 2102 2106 +4
|
Merging now. Making a release as well. @emdann I'll create an issue for your concern above. |
PR Checklist
docs
is updatedDescription of changes
Technical details
Additional context
Close #610