Conversation
| print( | ||
| f"Shrunk Log2 fold change & Wald test p-value: " | ||
| f"{self.contrast[0]} {self.contrast[1]} vs {self.contrast[2]}" | ||
| f"{split_coeff[0]} {split_coeff[1]} vs {split_coeff[3]}" |
There was a problem hiding this comment.
I got a bit lost, I was expecting split_coeff[2] instead of 3. can you write a short comment so it's easier to follow on what's in the coeff (or split_coeff) when reading the code?
There was a problem hiding this comment.
I've added a comment to explain (coeffs are of the form factor_A_vs_B, so because of the "vs" it's split_coeff[3] and not split_coeff[2]).
tests/test_edge_cases.py
Outdated
|
|
||
| res = DeseqStats(dds) | ||
| res.summary() | ||
| res.lfc_shrink(coeff=None) |
There was a problem hiding this comment.
would it be worth adding a test for other coeff values?
There was a problem hiding this comment.
I've updated the test to check that the same result is obtained with the implicit coeff condition_B_vs_A (the case where an incorrect coeff is provided is already covered)
|
Thanks for your review @maikia! Could you have a look at the updates I made and see if they answer the points you raised? |
maikia
left a comment
maikia
left a comment
There was a problem hiding this comment.
LGTM, thanks @BorisMuzellec
2 participants
Reference Issue or PRs
Fixes #121
What does your PR implement? Be specific.
This PR addresses #121 (wrong display of shrunk factors). More precisely:
lfc_shrinkwas fixed to match the coefficient that was actually shrunkref_levelinDeseqDataSetcoeff=Nonecoeff=Noneis tested intest_edge_cases.py