Merged
Conversation
added 3 commits
January 11, 2023 12:02
…ding on contrast, along with LFCs
…ctor tests and examples
…ads to coherent results
maikia
approved these changes
Jan 12, 2023
Collaborator
maikia
left a comment
There was a problem hiding this comment.
Thanks @BorisMuzellec nice PR. In general LGTM and ready to merge, just two minor points (not necessary for this PR, but good to keep in mind for the future):
- As part of this PR there are a lot of non-relevant (to this PR) name changes, etc.. which makes more difficult PR checking itself. Next time it would be great if you could keep those to separate PR.
- using your unit test I tried to make some tests on DeseqStats:
- when values:
res_B_vs_A = DeseqStats(dds, contrast=["condition", "A", "A"])are passed I get a message:*** KeyError: 'condition_A', perhaps more descriptive message could be given? res_B_vs_A = DeseqStats(dds, contrast=["group", "B", "A"])message is:*** AssertionError: The contrast levels should correspond to design factors levels.
Although this is correct a more descriptive message could be given (eg, design factor 'group' does not have 'A' level. The contrast levels should correspond to design factors levels.
Collaborator
Author
|
Thanks @maikia, I'll merge this PR then and open another one to improve error messages. |
Collaborator
Author
|
Hi @hfl112, can you try replacing underscores with hyphens?
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

This PR aims to debug
DeseqStats, which gave inconsistent results when changing the reference level using theconstrastargument.As an example, setting
constrast = ["condition", "A", "B"]instead ofconstrast = ["condition", "B", "A"]led to different p-values, whereas same p-values but opposite lfcs and stats are expected.More precisely, in this PR:
design_matrixattribute is added to theDeseqStatsclass,design_matrixandLFCfields of aDeseqStatsobject are consistent with itscontrastattribute,design_factors,test_contrastpytest was implemented to ensure that changing the reference level in the contrast yields expected results.