diff --git a/.gitignore b/.gitignore index bbe3c82e58..fb9e2a944b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ # Scanpy outfiles /data/ /write/ +/figures/ # Docs /docs/_build/ diff --git a/docs/release-notes/3675.misc.md b/docs/release-notes/3675.misc.md new file mode 100644 index 0000000000..31eb4535c3 --- /dev/null +++ b/docs/release-notes/3675.misc.md @@ -0,0 +1 @@ +Deprecate `save` parameter of plotting functions. {smaller}`zethson` diff --git a/notebooks b/notebooks index 8a5c354ef2..8e008841ed 160000 --- a/notebooks +++ b/notebooks @@ -1 +1 @@ -Subproject commit 8a5c354ef24ea1f233cfa15512df101f833bea09 +Subproject commit 8e008841edca909032186af8d46a4731476aa050 diff --git a/src/scanpy/plotting/_anndata.py b/src/scanpy/plotting/_anndata.py index b424a577de..0dea759944 100755 --- a/src/scanpy/plotting/_anndata.py +++ b/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. diff --git a/src/scanpy/plotting/_docs.py b/src/scanpy/plotting/_docs.py index c6b1d216be..490912682f 100644 --- a/src/scanpy/plotting/_docs.py +++ b/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"""\ diff --git a/src/scanpy/plotting/_preprocessing.py b/src/scanpy/plotting/_preprocessing.py index 349462b051..faf85719fd 100644 --- a/src/scanpy/plotting/_preprocessing.py +++ b/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. diff --git a/src/scanpy/plotting/_stacked_violin.py b/src/scanpy/plotting/_stacked_violin.py index 404605b5a2..133b64f61f 100644 --- a/src/scanpy/plotting/_stacked_violin.py +++ b/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. diff --git a/src/scanpy/plotting/_tools/__init__.py b/src/scanpy/plotting/_tools/__init__.py index 1f6a4fe3e1..cb5f6d1772 100644 --- a/src/scanpy/plotting/_tools/__init__.py +++ b/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). diff --git a/src/scanpy/plotting/_tools/paga.py b/src/scanpy/plotting/_tools/paga.py index bbaf3c4105..fb8d20b711 100644 --- a/src/scanpy/plotting/_tools/paga.py +++ b/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.""" diff --git a/src/scanpy/plotting/_tools/scatterplots.py b/src/scanpy/plotting/_tools/scatterplots.py index b68d703d41..4a26a45460 100644 --- a/src/scanpy/plotting/_tools/scatterplots.py +++ b/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. diff --git a/src/scanpy/plotting/_utils.py b/src/scanpy/plotting/_utils.py index be58b10baa..8016121cd4 100644 --- a/src/scanpy/plotting/_utils.py +++ b/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. diff --git a/tests/test_plotting_embedded/test_embeddings.py b/tests/test_plotting_embedded/test_embeddings.py index 82b89d7195..234f41753f 100644 --- a/tests/test_plotting_embedded/test_embeddings.py +++ b/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