Deprecate save parameter of plots

.gitignore

@@ -1,6 +1,7 @@
 # Scanpy outfiles
 /data/
 /write/
+/figures/
 
 # Docs
 /docs/_build/

docs/release-notes/3675.misc.md

@@ -0,0 +1 @@
+Deprecate `save` parameter of plotting functions. {smaller}`zethson`

src/scanpy/plotting/_anndata.py

@@ -147,8 +147,9 @@ def scatter(  # noqa: PLR0913
     marker: str | Sequence[str] = ".",
     title: str | Collection[str] | None = None,
     show: bool | None = None,
-    save: str | bool | None = None,
     ax: Axes | None = None,
+    # deprecated
+    save: str | bool | None = None,
 ) -> Axes | list[Axes] | None:
     """Scatter plot along observations or variables axes.
 
@@ -756,9 +757,9 @@ def violin(  # noqa: PLR0912, PLR0913, PLR0915
     ylabel: str | Sequence[str] | None = None,
     rotation: float | None = None,
     show: bool | None = None,
-    save: bool | str | None = None,
     ax: Axes | None = None,
-    # deprecatd
+    # deprecated
+    save: bool | str | None = None,
     scale: DensityNorm | Empty = _empty,
     **kwds,
 ) -> Axes | FacetGrid | None:
@@ -1013,7 +1014,7 @@ def clustermap(
     *,
     use_raw: bool | None = None,
     show: bool | None = None,
-    save: bool | str | None = None,
+    save: bool | str | None = None,  # deprecated
     **kwds,
 ) -> ClusterGrid | None:
     """Hierarchically-clustered heatmap.

src/scanpy/plotting/_docs.py

@@ -187,7 +187,8 @@
 save
     If `True` or a `str`, save the figure.
     A string is appended to the default filename.
-    Infer the filetype if ending on {`'.pdf'`, `'.png'`, `'.svg'`}.\
+    Infer the filetype if ending on {`'.pdf'`, `'.png'`, `'.svg'`}.
+    (deprecated in favour of `sc.pl.plot(show=False).figure.savefig()`).\
 """
 
 doc_show_save_ax = f"""\

src/scanpy/plotting/_preprocessing.py

@@ -21,8 +21,9 @@ def highly_variable_genes(  # noqa: PLR0912
     *,
     log: bool = False,
     show: bool | None = None,
-    save: bool | str | None = None,
     highly_variable_genes: bool = True,
+    # deprecated
+    save: bool | str | None = None,
 ) -> None:
     """Plot dispersions or normalized variance versus means for genes.
 
@@ -112,6 +113,7 @@ def filter_genes_dispersion(
     *,
     log: bool = False,
     show: bool | None = None,
+    # deprecated
     save: bool | str | None = None,
 ) -> None:
     """Plot dispersions versus means for genes.

src/scanpy/plotting/_stacked_violin.py

@@ -688,7 +688,6 @@ def stacked_violin(  # noqa: PLR0913
     categories_order: Sequence[str] | None = None,
     swap_axes: bool = False,
     show: bool | None = None,
-    save: bool | str | None = None,
     return_fig: bool | None = False,
     ax: _AxesSubplot | None = None,
     vmin: float | None = None,
@@ -706,6 +705,7 @@ def stacked_violin(  # noqa: PLR0913
     # deprecated
     order: Sequence[str] | None | Empty = _empty,
     scale: DensityNorm | Empty = _empty,
+    save: bool | str | None = None,
     **kwds,
 ) -> StackedViolin | dict | None:
     """Stacked violin plots.

src/scanpy/plotting/_tools/__init__.py

@@ -188,6 +188,7 @@ def pca_variance_ratio(
     *,
     log: bool = False,
     show: bool | None = None,
+    # deprecated
     save: bool | str | None = None,
 ):
     """Plot the variance ratio.
@@ -230,9 +231,10 @@ def dpt_timeseries(
     *,
     color_map: str | Colormap | None = None,
     show: bool | None = None,
-    save: bool | None = None,
     as_heatmap: bool = True,
     marker: str | Sequence[str] = ".",
+    # deprecated
+    save: bool | None = None,
 ):
     """Heatmap of pseudotime series.
 
@@ -277,8 +279,9 @@ def dpt_groups_pseudotime(
     color_map: str | Colormap | None = None,
     palette: Sequence[str] | Cycler | None = None,
     show: bool | None = None,
-    save: bool | str | None = None,
     marker: str | Sequence[str] = ".",
+    # deprecated
+    save: bool | str | None = None,
 ):
     """Plot groups and pseudotime.
 
@@ -346,8 +349,8 @@ def rank_genes_groups(  # noqa: PLR0912, PLR0913, PLR0915
     ncols: int = 4,
     sharey: bool = True,
     show: bool | None = None,
-    save: bool | None = None,
     ax: Axes | None = None,
+    save: bool | None = None,  # deprecated
     **kwds,
 ) -> list[Axes] | None:
     """Plot ranking of genes.
@@ -500,7 +503,12 @@ def rank_genes_groups(  # noqa: PLR0912, PLR0913, PLR0915
 
 
 def _fig_show_save_or_axes(
-    plot_obj: BasePlot, *, return_fig: bool, show: bool | None, save: bool | None
+    plot_obj: BasePlot,
+    *,
+    return_fig: bool,
+    show: bool | None,
+    # deprecated
+    save: bool | None,
 ):
     """Decides what to return."""
     if return_fig:
@@ -525,9 +533,9 @@ def _rank_genes_groups_plot(  # noqa: PLR0912, PLR0913, PLR0915
     min_logfoldchange: float | None = None,
     key: str | None = None,
     show: bool | None = None,
-    save: bool | None = None,
     return_fig: bool = False,
     gene_symbols: str | None = None,
+    save: bool | None = None,  # deprecated
     **kwds,
 ):
     """Call the different `rank_genes_groups_*` plots."""
@@ -700,7 +708,7 @@ def rank_genes_groups_heatmap(
     min_logfoldchange: float | None = None,
     key: str | None = None,
     show: bool | None = None,
-    save: bool | None = None,
+    save: bool | None = None,  # deprecated
     **kwds,
 ):
     """Plot ranking of genes using heatmap plot (see :func:`~scanpy.pl.heatmap`).
@@ -783,7 +791,7 @@ def rank_genes_groups_tracksplot(
     min_logfoldchange: float | None = None,
     key: str | None = None,
     show: bool | None = None,
-    save: bool | None = None,
+    save: bool | None = None,  # deprecated
     **kwds,
 ):
     """Plot ranking of genes using heatmap plot (see :func:`~scanpy.pl.heatmap`).
@@ -860,8 +868,8 @@ def rank_genes_groups_dotplot(  # noqa: PLR0913
     min_logfoldchange: float | None = None,
     key: str | None = None,
     show: bool | None = None,
-    save: bool | None = None,
     return_fig: bool = False,
+    save: bool | None = None,  # deprecated
     **kwds,
 ):
     """Plot ranking of genes using dotplot plot (see :func:`~scanpy.pl.dotplot`).
@@ -999,8 +1007,8 @@ def rank_genes_groups_stacked_violin(  # noqa: PLR0913
     min_logfoldchange: float | None = None,
     key: str | None = None,
     show: bool | None = None,
-    save: bool | None = None,
     return_fig: bool = False,
+    save: bool | None = None,  # deprecated
     **kwds,
 ):
     """Plot ranking of genes using stacked_violin plot.
@@ -1087,8 +1095,8 @@ def rank_genes_groups_matrixplot(  # noqa: PLR0913
     min_logfoldchange: float | None = None,
     key: str | None = None,
     show: bool | None = None,
-    save: bool | None = None,
     return_fig: bool = False,
+    save: bool | None = None,  # deprecated
     **kwds,
 ):
     """Plot ranking of genes using matrixplot plot (see :func:`~scanpy.pl.matrixplot`).
@@ -1227,8 +1235,8 @@ def rank_genes_groups_violin(  # noqa: PLR0913
     size: int = 1,
     ax: Axes | None = None,
     show: bool | None = None,
-    save: bool | None = None,
     # deprecated
+    save: bool | None = None,
     scale: DensityNorm | Empty = _empty,
 ):
     """Plot ranking of genes for all tested comparisons.
@@ -1345,8 +1353,9 @@ def sim(
     as_heatmap: bool = False,
     shuffle: bool = False,
     show: bool | None = None,
-    save: bool | str | None = None,
     marker: str | Sequence[str] = ".",
+    # deprecated
+    save: bool | str | None = None,
 ) -> None:
     """Plot results of simulation.
 
@@ -1453,9 +1462,9 @@ def embedding_density(  # noqa: PLR0912, PLR0913, PLR0915
     wspace: None = None,
     title: str | None = None,
     show: bool | None = None,
-    save: bool | str | None = None,
     ax: Axes | None = None,
     return_fig: bool | None = None,
+    save: bool | str | None = None,  # deprecated
     **kwargs,
 ) -> Figure | Axes | None:
     """Plot the density of cells in an embedding (per condition).

src/scanpy/plotting/_tools/paga.py

@@ -340,8 +340,9 @@ def paga(  # noqa: PLR0912, PLR0913, PLR0915
     groups=None,  # backwards compat
     plot: bool = True,
     show: bool | None = None,
-    save: bool | str | None = None,
     ax: Axes | None = None,
+    # deprecated
+    save: bool | str | None = None,
 ) -> Axes | list[Axes] | None:
     r"""Plot the PAGA graph through thresholding low-connectivity edges.
 
@@ -1067,8 +1068,9 @@ def paga_path(  # noqa: PLR0912, PLR0913, PLR0915
     as_heatmap: bool = True,
     return_data: bool = False,
     show: bool | None = None,
-    save: bool | str | None = None,
     ax: Axes | None = None,
+    # deprecated
+    save: bool | str | None = None,
 ) -> tuple[Axes, pd.DataFrame] | Axes | pd.DataFrame | None:
     r"""Gene expression and annotation changes along paths in the abstracted graph.
 
@@ -1363,6 +1365,7 @@ def paga_adjacency(
     as_heatmap: bool = True,
     color_map: str | Colormap | None = None,
     show: bool | None = None,
+    # deprecated
     save: bool | str | None = None,
 ) -> None:
     """Plot connectivity of paga groups."""

src/scanpy/plotting/_tools/scatterplots.py

@@ -112,10 +112,10 @@ def embedding(  # noqa: PLR0912, PLR0913, PLR0915
     wspace: float | None = None,
     title: str | Sequence[str] | None = None,
     show: bool | None = None,
-    save: bool | str | None = None,
     ax: Axes | None = None,
     return_fig: bool | None = None,
     marker: str | Sequence[str] = ".",
+    save: bool | str | None = None,  # deprecated
     **kwargs,
 ) -> Figure | Axes | list[Axes] | None:
     """Scatter plot for user specified embedding basis (e.g. umap, pca, etc).
@@ -831,7 +831,7 @@ def pca(
     annotate_var_explained: bool = False,
     show: bool | None = None,
     return_fig: bool | None = None,
-    save: bool | str | None = None,
+    save: bool | str | None = None,  # deprecated
     **kwargs,
 ) -> Figure | Axes | list[Axes] | None:
     """Scatter plot in PCA coordinates.
@@ -946,7 +946,7 @@ def spatial(  # noqa: PLR0913
     na_color: ColorLike | None = None,
     show: bool | None = None,
     return_fig: bool | None = None,
-    save: bool | str | None = None,
+    save: bool | str | None = None,  # deprecated
     **kwargs,
 ) -> Figure | Axes | list[Axes] | None:
     """Scatter plot in spatial coordinates.

src/scanpy/plotting/_utils.py

@@ -16,7 +16,7 @@
 from matplotlib.patches import Circle
 
 from .. import logging as logg
-from .._compat import old_positionals
+from .._compat import deprecated, old_positionals
 from .._settings import settings
 from .._utils import NeighborsView, _empty
 from . import palettes
@@ -94,8 +94,9 @@ def matrix(  # noqa: PLR0913
     colorbar_shrink: float = 0.5,
     color_map: str | Colormap | None = None,
     show: bool | None = None,
-    save: bool | str | None = None,
     ax: Axes | None = None,
+    # deprecated
+    save: bool | str | None = None,
 ) -> None:
     """Plot a matrix."""
     if ax is None:
@@ -306,6 +307,9 @@ def timeseries_as_heatmap(
 # -------------------------------------------------------------------------------
 
 
+@deprecated(
+    "Argument `save` is deprecated and will be removed in a future version. Use `sc.pl.plot(show=False).figure.savefig()` instead."
+)
 def savefig(writekey, dpi=None, ext=None):
     """Save current figure to file.
 

tests/test_plotting_embedded/test_embeddings.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import tempfile
+import uuid
 from functools import partial, wraps
 from pathlib import Path
 from typing import TYPE_CHECKING
@@ -251,3 +253,26 @@ def test_embedding_colorbar_location(image_comparer):
     sc.pl.pca(adata, color="LDHB", colorbar_loc=None)
 
     save_and_compare_images("no_colorbar")
+
+
+def test_raise_save_future_warning():
+    adata = pbmc3k_processed()
+
+    unique_id = str(uuid.uuid4())[:8]
+    test_filename = f"test_violin_{unique_id}.png"
+
+    with tempfile.TemporaryDirectory() as temp_dir:
+        original_figdir = sc.settings.figdir
+        sc.settings.figdir = Path(temp_dir)
+
+        try:
+            with pytest.warns(FutureWarning, match=r"Argument `save` is deprecated"):
+                sc.pl.violin(adata, keys="louvain", save=test_filename, show=False)
+
+            expected_file = Path(temp_dir) / f"violin{test_filename}"
+            assert expected_file.exists(), (
+                f"Expected file {expected_file} was not created"
+            )
+
+        finally:
+            sc.settings.figdir = original_figdir