Skip to content

add plotMA() functionality#145

Merged
BorisMuzellec merged 10 commits intoscverse:mainfrom
emmcauley:feat_MAplot
Jul 25, 2023
Merged

add plotMA() functionality#145
BorisMuzellec merged 10 commits intoscverse:mainfrom
emmcauley:feat_MAplot

Conversation

@emmcauley
Copy link
Copy Markdown
Contributor

Reference Issue or PRs

There is no existing issue that I saw that is requesting this functionality.

What does your PR implement? Be specific.

This PR uses matplotlib to implement a plotMA() function.
The MA plot shows the mean of the normalized counts versus the log2 fold changes for all genes tested. Genes that are significantly differentially expressed (based on some user-specified threshold) are colored red; the non-significant genes are colored grey. This plot can be helpful to look at LFC shrinkage effects.

This functionality tries to copy the version of plotMA() in R. Example:
image

A couple of considerations I wanted to flag:

  1. Rows that return NaN in padj are colored grey and ultimately ignored (let me know if you'd like me to change this behavior, I think in R they are also ignored?)
  2. I modify the results_df to add a new column, color. This may not be preferable, and I'd be grateful for input to clean this up.
  3. There is some overlap between this and make_scatter() -- if it's helpful, I could potentially generalize make_scatter() to work with plot_dispersions() and MAplot()

@emmcauley emmcauley requested a review from a user June 23, 2023 22:38
@ghost
Copy link
Copy Markdown

ghost commented Jun 29, 2023

hello @emmcauley , thanks a lot for your PR!

The tests are currently not passing due to the lack of an extended summary in the numpy doc style, meaning a longer sentence than the current description. Can you please add one?

Regarding the actual content, we are a bit short on staff at the moment, but we'll come back to you in a few weeks!

@emmcauley
Copy link
Copy Markdown
Contributor Author

@mandreux-owkin, thanks for this context -- I've updated the extended summary and it looks like the checks are passing now. Let me know if you need anything else and/or want to see any changes before merging.

@BorisMuzellec
Copy link
Copy Markdown
Collaborator

Hi @emmcauley, thanks a lot for this new PR, and sorry for replying so late!

I made a couple of changes:

  • I changed the code in utils.py to avoid adding a column to results_df with the colors,
  • I added a wrapper method in DeseqStats,
  • I added a pytest,
  • and I did a bit of refactoring here and there.

Happy to merge if you're happy with the current state of this PR.

@emmcauley
Copy link
Copy Markdown
Contributor Author

Sorry for the delay, thanks @BorisMuzellec, looks great! I think the use of p < thresh is more consistent with the R documentation (as opposed to p <= thresh).

@BorisMuzellec BorisMuzellec merged commit 51f2eae into scverse:main Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants