Add plots for DE analysis and unify plotting API #654

Lilly-May · 2024-09-05T20:21:20Z

PR Checklist

Referenced issue is linked (Better support for DE in scverse #189)
Documentation in docs is updated

Description of changes

Unified plotting API: We now have three parameters (save, show, return_fig) in the plotting functions, and I added a utility method to handle showing and saving plots. Previously, showing and storing figures were handled differently ba the different plotting functions.
Added four plotting functions to be used in the DE tutorial: one for pseudobulk analysis, and three for differential expression analysis, as described in Better support for DE in scverse #189.

Discussion points
I'm not entirely sure about the plot_fold_change method yet. In #189, it was suggested as a plot for paired samples, but I think it would be nice (and I implemented it that way as of now) if the plot could work directly with the results_df returned by all DE tools, i.e., plotting the log-FC from that dataframe. However, I’m unsure how to incorporate the pairing in that case and would appreciate ideas.

# Conflicts: # pertpy/tools/__init__.py

# Conflicts: # .github/workflows/test.yml

codecov-commenter · 2024-09-06T10:54:47Z

Codecov Report

Attention: Patch coverage is 26.24434% with 163 lines in your changes missing coverage. Please review.

Project coverage is 65.55%. Comparing base (2ad41a7) to head (7e814fa).

Files with missing lines	Patch %	Lines
...ertpy/tools/_differential_gene_expression/_base.py	12.30%	114 Missing ⚠️
pertpy/tools/_milo.py	36.84%	12 Missing ⚠️
pertpy/preprocessing/_guide_rna.py	30.00%	7 Missing ⚠️
pertpy/tools/_perturbation_space/_simple.py	46.15%	7 Missing ⚠️
pertpy/metadata/_cell_line.py	25.00%	6 Missing ⚠️
pertpy/tools/_cinemaot.py	25.00%	6 Missing ⚠️
pertpy/tools/_enrichment.py	33.33%	6 Missing ⚠️
pertpy/tools/_scgen/_scgen.py	40.00%	3 Missing ⚠️
pertpy/tools/_dialogue.py	75.00%	1 Missing ⚠️
pertpy/tools/_mixscape.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #654      +/-   ##
==========================================
- Coverage   66.34%   65.55%   -0.79%     
==========================================
  Files          46       47       +1     
  Lines        5960     6105     +145     
==========================================
+ Hits         3954     4002      +48     
- Misses       2006     2103      +97

Files with missing lines	Coverage Δ
pertpy/_doc.py	`100.00% <100.00%> (ø)`
pertpy/tools/__init__.py	`85.71% <100.00%> (ø)`
pertpy/tools/_augur.py	`72.15% <100.00%> (+3.47%)`	⬆️
pertpy/tools/_coda/_base_coda.py	`56.30% <100.00%> (+0.07%)`	⬆️
...rtpy/tools/_differential_gene_expression/_edger.py	`87.09% <100.00%> (ø)`
...y/tools/_perturbation_space/_perturbation_space.py	`86.59% <ø> (ø)`
pertpy/tools/_dialogue.py	`35.47% <75.00%> (+1.21%)`	⬆️
pertpy/tools/_mixscape.py	`79.12% <50.00%> (-0.29%)`	⬇️
pertpy/tools/_scgen/_scgen.py	`63.69% <40.00%> (+0.42%)`	⬆️
pertpy/metadata/_cell_line.py	`49.80% <25.00%> (-0.60%)`	⬇️
... and 6 more

Zethson

Thank you very much! So many improvements in this PR.

are the stars missing for the significance levels here?
_utils modules/files are a code smell. Could we move the savefig_or_show code into a plotting module? Could we move the doc stuff into a _doc file or something?
I didn't always want to mark it, but show: if True, show to the plot after saving it. is maybe better wording then what you have? Another set of colons can also cause unintended issues.
Could you amend the tutorial, please?

pertpy/_utils.py

pertpy/tools/_perturbation_space/_simple.py

tests/tools/_differential_gene_expression/test_edger.py

pertpy/tools/_differential_gene_expression/_base.py

Lilly-May · 2024-09-06T20:54:49Z

Thanks for the review!

The stars were there, but in white... Not my smartest move... It's fixed now.
Sure, I'll move both. But for the plotting utilities, are you planning to keep the pertpy.plot module in the long run, even though all the plots have been moved to the tools module? In that case, maybe I shouldn't move it into that module.
Not really sure what you're referring to here. Do you mean the default parameter description in _utils?
Yes, I already started adapting the tutorial and hope to get it done over the weekend.

Co-authored-by: Lukas Heumos <[email protected]>

pertpy/tools/_scgen/_scgen.py

Zethson · 2024-09-06T20:58:59Z

We may redo the architecture again in the future if I ever find the time. But for now, we don't plan on providing a lot if anything in the pl plotting module.
Added a comment

Lilly-May · 2024-09-07T07:48:07Z

Hi @grst!
We've added some DE plots in this PR following your suggestions in #189. However, I'm unsure how to implement the paired fold-change bar plot without completely ignoring the results from the DE tools, i.e. I would like to use the results_df as input to the plot, as it is currently implemented in this PR. Do you maybe have any suggestions? Thanks in advance!

pertpy/_utils.py

pertpy/tools/_differential_gene_expression/_base.py

grst · 2024-09-08T18:44:50Z

I would like to use the results_df as input to the plot, as it is currently implemented in this PR. Do you maybe have any suggestions?

The "best" solution IMO would be the Results class object as indicated in the code comments. But since this might entail quite some additional refactoring this is probably beyond the scope of this PR. Why can't you just provide the data frame as you suggest and then filter it for the relevant genes?

Lilly-May · 2024-09-09T11:28:09Z

Why can't you just provide the data frame as you suggest and then filter it for the relevant genes?

If I understand you correctly, that's what I'm currently doing. I'm just not sure how to incorporate the pairing into this based on the results_df alone, without calculating a lot of new metrics. But maybe the current plot version is sufficient for this PR, and we can consider adding the one including paired samples as a separate plot in a future PR.

pertpy/tools/_perturbation_space/_simple.py

grst · 2024-09-09T14:36:13Z

pertpy/tools/_differential_gene_expression/_base.py

@@ -243,7 +299,7 @@ def _map_genes_categories_highlight(
            if varm_key is None:
                raise ValueError("Please pass a .varm key to use for plotting")

-            raise NotImplementedError("Anndata not implemented yet")
+            raise NotImplementedError("Anndata not implemented yet")  # TODO: Implement this


What would be the anndata usecase here actually? DE results stored in .varm?
This is not supported by the DE tools either, so not sure this is required.

I'm actually not sure myself - that part was already implemented (or rather not implemented in this case). @Zethson, what exactly did you have in mind here? Should I remove it?

pertpy/tools/_differential_gene_expression/_base.py

grst

Really nice work btw, thank you for working on this!

pertpy/tools/_differential_gene_expression/_base.py

grst

A few more minor points... LGTM in general now

pertpy/preprocessing/_guide_rna.py

pertpy/tools/_augur.py

pertpy/tools/_cinemaot.py

pertpy/tools/_coda/_base_coda.py

pertpy/tools/_differential_gene_expression/_base.py

Lilly-May added 9 commits August 31, 2024 23:05

EdgeR example and first paired plot implementation

0905ff2

Merge branch 'main' into feature/de_improvements

ab949d1

# Conflicts: # pertpy/tools/__init__.py

Reuse plot parameters

8881261

Added DE plotting functionalities

32f878e

Added plot previews

914b95c

Unify plotting API

dda5a35

Updated all plots to new API

8616d78

Resolved ToDos

15ebd8f

Merge branch 'main' into feature/de_improvements

378121d

# Conflicts: # .github/workflows/test.yml

github-actions bot added the enhancement New feature or request label Sep 5, 2024

Coda plots docs with dict

fce6249

Final coda docs edits

faa09d3

Lilly-May marked this pull request as ready for review September 6, 2024 14:22

Lilly-May requested a review from Zethson September 6, 2024 14:22

Zethson approved these changes Sep 6, 2024

View reviewed changes

Plot stars in legend in black

7753a9f

Lilly-May and others added 3 commits September 6, 2024 22:56

Apply suggestions from code review

fc61f88

Co-authored-by: Lukas Heumos <[email protected]>

Update pertpy/tools/_differential_gene_expression/_base.py

0d59f41

Co-authored-by: Lukas Heumos <[email protected]>

Update pertpy/tools/_differential_gene_expression/_base.py

4ef0295

Co-authored-by: Lukas Heumos <[email protected]>

Zethson reviewed Sep 6, 2024

View reviewed changes

pertpy/tools/_scgen/_scgen.py Outdated Show resolved Hide resolved

Lilly-May requested a review from grst September 7, 2024 07:48

grst reviewed Sep 8, 2024

View reviewed changes

pertpy/_utils.py Outdated Show resolved Hide resolved

pertpy/tools/_differential_gene_expression/_base.py Outdated Show resolved Hide resolved

Lilly-May added 2 commits September 9, 2024 14:22

Removed all save parameters

910226a

Removed _utils.py file

6d1ffe8

Fixed edgeR test

0ddfe79

grst reviewed Sep 9, 2024

View reviewed changes

pertpy/tools/_differential_gene_expression/_base.py Outdated Show resolved Hide resolved

Use p-values from results_df in plot_paired

5a2e14d

Lilly-May mentioned this pull request Sep 9, 2024

Showcase new plots in Differential Expression tutorial scverse/pertpy-tutorials#45

Merged

grst reviewed Sep 10, 2024

View reviewed changes

Lilly-May added 2 commits September 10, 2024 11:30

PR Reviews

407df8c

Pull submodule

7e814fa

grst approved these changes Sep 10, 2024

View reviewed changes

Zethson merged commit 99dcd18 into main Sep 16, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add plots for DE analysis and unify plotting API #654

Add plots for DE analysis and unify plotting API #654

Lilly-May commented Sep 5, 2024

codecov-commenter commented Sep 6, 2024 •

edited

Loading

Zethson left a comment

Lilly-May commented Sep 6, 2024

Zethson commented Sep 6, 2024

Lilly-May commented Sep 7, 2024

grst commented Sep 8, 2024

Lilly-May commented Sep 9, 2024

grst Sep 9, 2024

Lilly-May Sep 9, 2024

grst left a comment

grst left a comment

Add plots for DE analysis and unify plotting API #654

Add plots for DE analysis and unify plotting API #654

Conversation

Lilly-May commented Sep 5, 2024

codecov-commenter commented Sep 6, 2024 • edited Loading

Codecov Report

Zethson left a comment

Choose a reason for hiding this comment

Lilly-May commented Sep 6, 2024

Zethson commented Sep 6, 2024

Lilly-May commented Sep 7, 2024

grst commented Sep 8, 2024

Lilly-May commented Sep 9, 2024

grst Sep 9, 2024

Choose a reason for hiding this comment

Lilly-May Sep 9, 2024

Choose a reason for hiding this comment

grst left a comment

Choose a reason for hiding this comment

grst left a comment

Choose a reason for hiding this comment

codecov-commenter commented Sep 6, 2024 •

edited

Loading