From bead0121806c455e6f9cdb4fa321e1a36c465dc2 Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Wed, 23 Nov 2022 22:15:59 +0100 Subject: [PATCH 01/12] Use plt.rc_context for default styles --- xarray/plot/dataarray_plot.py | 54 +++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/xarray/plot/dataarray_plot.py b/xarray/plot/dataarray_plot.py index ac23f7dc96d..d62d6bd23d1 100644 --- a/xarray/plot/dataarray_plot.py +++ b/xarray/plot/dataarray_plot.py @@ -62,6 +62,11 @@ ) from .facetgrid import FacetGrid +_styles: MutableMapping[str, Any] = { + # Add a white border to make it easier seeing overlapping markers: + "scatter.edgecolors": "w", +} + def _infer_line_data( darray: DataArray, x: Hashable | None, y: Hashable | None, hue: Hashable | None @@ -938,6 +943,8 @@ def newplotfunc( warnings.warn(msg, DeprecationWarning, stacklevel=2) del args + plt = import_matplotlib_pyplot() + _is_facetgrid = kwargs.pop("_is_facetgrid", False) if plotfunc.__name__ == "scatter": @@ -984,29 +991,29 @@ def newplotfunc( ckw = {vv: cmap_params[vv] for vv in ("vmin", "vmax", "norm", "cmap")} cmap_params_subset.update(**ckw) - if z is not None: - if ax is None: - subplot_kws.update(projection="3d") - ax = get_axis(figsize, size, aspect, ax, **subplot_kws) - # Using 30, 30 minimizes rotation of the plot. Making it easier to - # build on your intuition from 2D plots: - plt = import_matplotlib_pyplot() - if Version(plt.matplotlib.__version__) < Version("3.5.0"): - ax.view_init(azim=30, elev=30) + with plt.rc_context(_styles): + if z is not None: + if ax is None: + subplot_kws.update(projection="3d") + ax = get_axis(figsize, size, aspect, ax, **subplot_kws) + # Using 30, 30 minimizes rotation of the plot. Making it easier to + # build on your intuition from 2D plots: + if Version(plt.matplotlib.__version__) < Version("3.5.0"): + ax.view_init(azim=30, elev=30) + else: + # https://github.com/matplotlib/matplotlib/pull/19873 + ax.view_init(azim=30, elev=30, vertical_axis="y") else: - # https://github.com/matplotlib/matplotlib/pull/19873 - ax.view_init(azim=30, elev=30, vertical_axis="y") - else: - ax = get_axis(figsize, size, aspect, ax, **subplot_kws) - - primitive = plotfunc( - xplt, - yplt, - ax=ax, - add_labels=add_labels, - **cmap_params_subset, - **kwargs, - ) + ax = get_axis(figsize, size, aspect, ax, **subplot_kws) + + primitive = plotfunc( + xplt, + yplt, + ax=ax, + add_labels=add_labels, + **cmap_params_subset, + **kwargs, + ) if np.any(np.asarray(add_labels)) and add_title: ax.set_title(darray._title_for_slice()) @@ -1251,9 +1258,6 @@ def scatter( hueplt: DataArray | None = kwargs.pop("hueplt", None) sizeplt: DataArray | None = kwargs.pop("sizeplt", None) - # Add a white border to make it easier seeing overlapping markers: - kwargs.update(edgecolors=kwargs.pop("edgecolors", "w")) - if hueplt is not None: kwargs.update(c=hueplt.to_numpy().ravel()) From 82c86fef1f6b407e92e5f1899d6d90be0103d6f0 Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Wed, 23 Nov 2022 23:10:05 +0100 Subject: [PATCH 02/12] Add tests --- xarray/tests/test_plot.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 88f03c03b40..a70cc7b63c3 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -6,6 +6,7 @@ from copy import copy from datetime import datetime from typing import Any, Callable, Hashable, Literal +import warnings import numpy as np import pandas as pd @@ -3220,3 +3221,21 @@ def test_facetgrid_axes_raises_deprecation_warning() -> None: ds = xr.tutorial.scatter_example_dataset() g = ds.plot.scatter(x="A", y="B", col="x") g.axes + + +@requires_matplotlib +def test_scatter_edgecolor() -> None: + import matplotlib as mpl + + ds = xr.Dataset({"a": ("dim", np.arange(3, 10))}, {"dim": np.arange(7)}) + + with figure_context(): + fig, ax = plt.subplots(1, 1) + ds.plot.scatter(x="dim", y="a", marker="o", ax=ax) + np.testing.assert_allclose( + ax.collections[0].get_edgecolor(), mpl.colors.to_rgba_array("w") + ) + + with warnings.catch_warnings(): + warnings.simplefilter("error") + ds.plot.scatter(x="dim", y="a", marker="x") From 091367b29399dcf6dd61772fb6ad9388899233e9 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 23 Nov 2022 22:12:34 +0000 Subject: [PATCH 03/12] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- xarray/tests/test_plot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index a70cc7b63c3..dec6cc01c74 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -3,10 +3,10 @@ import contextlib import inspect import math +import warnings from copy import copy from datetime import datetime from typing import Any, Callable, Hashable, Literal -import warnings import numpy as np import pandas as pd From 34b909440b33079e01ea77083b2f29c79f41839c Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Thu, 24 Nov 2022 07:11:25 +0100 Subject: [PATCH 04/12] Explain tests --- xarray/tests/test_plot.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index a70cc7b63c3..4d20d0dfc16 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -3230,12 +3230,15 @@ def test_scatter_edgecolor() -> None: ds = xr.Dataset({"a": ("dim", np.arange(3, 10))}, {"dim": np.arange(7)}) with figure_context(): + # scatter markers should by default have white edgecolor to better + # see overlapping markers: fig, ax = plt.subplots(1, 1) ds.plot.scatter(x="dim", y="a", marker="o", ax=ax) np.testing.assert_allclose( ax.collections[0].get_edgecolor(), mpl.colors.to_rgba_array("w") ) + # scatter should not emit any warnings when using unfilled markers: with warnings.catch_warnings(): warnings.simplefilter("error") ds.plot.scatter(x="dim", y="a", marker="x") From 5fd611f99171a324af62f9a3171d8ff944154095 Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Sat, 26 Nov 2022 15:51:24 +0100 Subject: [PATCH 05/12] Check that args are prioritized --- xarray/tests/test_plot.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 17983c4566f..65e599870c5 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -3224,7 +3224,7 @@ def test_facetgrid_axes_raises_deprecation_warning() -> None: @requires_matplotlib -def test_scatter_edgecolor() -> None: +def test_plot1d_default_rcparams() -> None: import matplotlib as mpl ds = xr.Dataset({"a": ("dim", np.arange(3, 10))}, {"dim": np.arange(7)}) @@ -3242,3 +3242,10 @@ def test_scatter_edgecolor() -> None: with warnings.catch_warnings(): warnings.simplefilter("error") ds.plot.scatter(x="dim", y="a", marker="x") + + # Prioritize edgecolor argument over default plot1d values: + fig, ax = plt.subplots(1, 1) + ds.plot.scatter(x="dim", y="a", marker="o", ax=ax, edgecolor="k") + np.testing.assert_allclose( + ax.collections[0].get_edgecolor(), mpl.colors.to_rgba_array("k") + ) From 121f724541cd5e0d43f709dd3c24569ba1564d53 Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Sat, 26 Nov 2022 15:56:47 +0100 Subject: [PATCH 06/12] Use rc_context as decorator --- xarray/plot/dataarray_plot.py | 48 +++++++++++++++++------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/xarray/plot/dataarray_plot.py b/xarray/plot/dataarray_plot.py index d62d6bd23d1..53923d5a5b0 100644 --- a/xarray/plot/dataarray_plot.py +++ b/xarray/plot/dataarray_plot.py @@ -851,6 +851,7 @@ def _plot1d(plotfunc): The same type of primitive artist that the wrapped matplotlib function returns """ + plt = import_matplotlib_pyplot() # Build on the original docstring plotfunc.__doc__ = f"{plotfunc.__doc__}\n{commondoc}" @@ -858,6 +859,7 @@ def _plot1d(plotfunc): @functools.wraps( plotfunc, assigned=("__module__", "__name__", "__qualname__", "__doc__") ) + @plt.rc_context(_styles) def newplotfunc( darray: DataArray, *args: Any, @@ -910,6 +912,7 @@ def newplotfunc( allargs = locals().copy() allargs.update(allargs.pop("kwargs")) allargs.pop("darray") + allargs.pop("plt") allargs["plotfunc"] = globals()[plotfunc.__name__] return _easy_facetgrid(darray, kind="plot1d", **allargs) @@ -943,8 +946,6 @@ def newplotfunc( warnings.warn(msg, DeprecationWarning, stacklevel=2) del args - plt = import_matplotlib_pyplot() - _is_facetgrid = kwargs.pop("_is_facetgrid", False) if plotfunc.__name__ == "scatter": @@ -991,29 +992,28 @@ def newplotfunc( ckw = {vv: cmap_params[vv] for vv in ("vmin", "vmax", "norm", "cmap")} cmap_params_subset.update(**ckw) - with plt.rc_context(_styles): - if z is not None: - if ax is None: - subplot_kws.update(projection="3d") - ax = get_axis(figsize, size, aspect, ax, **subplot_kws) - # Using 30, 30 minimizes rotation of the plot. Making it easier to - # build on your intuition from 2D plots: - if Version(plt.matplotlib.__version__) < Version("3.5.0"): - ax.view_init(azim=30, elev=30) - else: - # https://github.com/matplotlib/matplotlib/pull/19873 - ax.view_init(azim=30, elev=30, vertical_axis="y") + if z is not None: + if ax is None: + subplot_kws.update(projection="3d") + ax = get_axis(figsize, size, aspect, ax, **subplot_kws) + # Using 30, 30 minimizes rotation of the plot. Making it easier to + # build on your intuition from 2D plots: + if Version(plt.matplotlib.__version__) < Version("3.5.0"): + ax.view_init(azim=30, elev=30) else: - ax = get_axis(figsize, size, aspect, ax, **subplot_kws) - - primitive = plotfunc( - xplt, - yplt, - ax=ax, - add_labels=add_labels, - **cmap_params_subset, - **kwargs, - ) + # https://github.com/matplotlib/matplotlib/pull/19873 + ax.view_init(azim=30, elev=30, vertical_axis="y") + else: + ax = get_axis(figsize, size, aspect, ax, **subplot_kws) + + primitive = plotfunc( + xplt, + yplt, + ax=ax, + add_labels=add_labels, + **cmap_params_subset, + **kwargs, + ) if np.any(np.asarray(add_labels)) and add_title: ax.set_title(darray._title_for_slice()) From 509cf5f34836a282456d7ffbe848c6dd31ca034b Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Sat, 26 Nov 2022 16:10:42 +0100 Subject: [PATCH 07/12] Using as decorator is unfortunately not lazy, reverting --- xarray/plot/dataarray_plot.py | 47 ++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/xarray/plot/dataarray_plot.py b/xarray/plot/dataarray_plot.py index 53923d5a5b0..d28f9d8ed20 100644 --- a/xarray/plot/dataarray_plot.py +++ b/xarray/plot/dataarray_plot.py @@ -851,7 +851,6 @@ def _plot1d(plotfunc): The same type of primitive artist that the wrapped matplotlib function returns """ - plt = import_matplotlib_pyplot() # Build on the original docstring plotfunc.__doc__ = f"{plotfunc.__doc__}\n{commondoc}" @@ -859,7 +858,6 @@ def _plot1d(plotfunc): @functools.wraps( plotfunc, assigned=("__module__", "__name__", "__qualname__", "__doc__") ) - @plt.rc_context(_styles) def newplotfunc( darray: DataArray, *args: Any, @@ -901,6 +899,8 @@ def newplotfunc( # All 1d plots in xarray share this function signature. # Method signature below should be consistent. + plt = import_matplotlib_pyplot() + if subplot_kws is None: subplot_kws = dict() @@ -992,28 +992,29 @@ def newplotfunc( ckw = {vv: cmap_params[vv] for vv in ("vmin", "vmax", "norm", "cmap")} cmap_params_subset.update(**ckw) - if z is not None: - if ax is None: - subplot_kws.update(projection="3d") - ax = get_axis(figsize, size, aspect, ax, **subplot_kws) - # Using 30, 30 minimizes rotation of the plot. Making it easier to - # build on your intuition from 2D plots: - if Version(plt.matplotlib.__version__) < Version("3.5.0"): - ax.view_init(azim=30, elev=30) + with plt.rc_context(_styles): + if z is not None: + if ax is None: + subplot_kws.update(projection="3d") + ax = get_axis(figsize, size, aspect, ax, **subplot_kws) + # Using 30, 30 minimizes rotation of the plot. Making it easier to + # build on your intuition from 2D plots: + if Version(plt.matplotlib.__version__) < Version("3.5.0"): + ax.view_init(azim=30, elev=30) + else: + # https://github.com/matplotlib/matplotlib/pull/19873 + ax.view_init(azim=30, elev=30, vertical_axis="y") else: - # https://github.com/matplotlib/matplotlib/pull/19873 - ax.view_init(azim=30, elev=30, vertical_axis="y") - else: - ax = get_axis(figsize, size, aspect, ax, **subplot_kws) - - primitive = plotfunc( - xplt, - yplt, - ax=ax, - add_labels=add_labels, - **cmap_params_subset, - **kwargs, - ) + ax = get_axis(figsize, size, aspect, ax, **subplot_kws) + + primitive = plotfunc( + xplt, + yplt, + ax=ax, + add_labels=add_labels, + **cmap_params_subset, + **kwargs, + ) if np.any(np.asarray(add_labels)) and add_title: ax.set_title(darray._title_for_slice()) From 86ee189f08107b16c04054e3dc940648612aa4b7 Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Sat, 26 Nov 2022 16:31:04 +0100 Subject: [PATCH 08/12] Add test for facetgrid as well --- xarray/tests/test_plot.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 65e599870c5..0c42ace36c0 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -3227,13 +3227,20 @@ def test_facetgrid_axes_raises_deprecation_warning() -> None: def test_plot1d_default_rcparams() -> None: import matplotlib as mpl - ds = xr.Dataset({"a": ("dim", np.arange(3, 10))}, {"dim": np.arange(7)}) + ds = xr.tutorial.scatter_example_dataset(seed=42) with figure_context(): # scatter markers should by default have white edgecolor to better # see overlapping markers: fig, ax = plt.subplots(1, 1) - ds.plot.scatter(x="dim", y="a", marker="o", ax=ax) + ds.plot.scatter(x="A", y="B", marker="o", ax=ax) + np.testing.assert_allclose( + ax.collections[0].get_edgecolor(), mpl.colors.to_rgba_array("w") + ) + + # Facetgrids should have the default value as well: + fg = ds.plot.scatter(x="A", y="B", col="x", marker="o") + ax = fg.axs.ravel()[0] np.testing.assert_allclose( ax.collections[0].get_edgecolor(), mpl.colors.to_rgba_array("w") ) @@ -3241,11 +3248,12 @@ def test_plot1d_default_rcparams() -> None: # scatter should not emit any warnings when using unfilled markers: with warnings.catch_warnings(): warnings.simplefilter("error") - ds.plot.scatter(x="dim", y="a", marker="x") + fig, ax = plt.subplots(1, 1) + ds.plot.scatter(x="A", y="B", ax=ax, marker="x") # Prioritize edgecolor argument over default plot1d values: fig, ax = plt.subplots(1, 1) - ds.plot.scatter(x="dim", y="a", marker="o", ax=ax, edgecolor="k") + ds.plot.scatter(x="A", y="B", marker="o", ax=ax, edgecolor="k") np.testing.assert_allclose( ax.collections[0].get_edgecolor(), mpl.colors.to_rgba_array("k") ) From cb41283a19655888dc598abfbfc484f2e50a462d Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Sat, 26 Nov 2022 16:59:25 +0100 Subject: [PATCH 09/12] Update whats-new.rst --- doc/whats-new.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 13867daebf4..0b280766b5d 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -56,6 +56,9 @@ Bug fixes - Import ``nc_time_axis`` when needed (:issue:`7275`, :pull:`7276`). By `Michael Niklas `_. +- Fix matplotlib raising a UserWarning when plotting a scatter plot + with an unfilled marker (:issue:`7313`, :pull:`7318`). + By `Jimmy Westling `_. Documentation ~~~~~~~~~~~~~ From b188b176ea5a0eee122ce285400136b320216647 Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Sat, 26 Nov 2022 17:01:48 +0100 Subject: [PATCH 10/12] Update whats-new.rst --- doc/whats-new.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 7a2c06960ea..38fa79e76dc 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -57,11 +57,11 @@ Bug fixes - Import ``nc_time_axis`` when needed (:issue:`7275`, :pull:`7276`). By `Michael Niklas `_. +- Fix static typing of :py:meth:`xr.polyval` (:issue:`7312`, :pull:`7315`). + By `Michael Niklas `_. - Fix matplotlib raising a UserWarning when plotting a scatter plot with an unfilled marker (:issue:`7313`, :pull:`7318`). By `Jimmy Westling `_. -- Fix static typing of :py:meth:`xr.polyval` (:issue:`7312`, :pull:`7315`). - By `Michael Niklas `_. Documentation ~~~~~~~~~~~~~ From fd6b69f62dce112b52986c37221f65a5d5b05201 Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Tue, 29 Nov 2022 19:01:36 +0100 Subject: [PATCH 11/12] Update xarray/tests/test_plot.py Co-authored-by: Mathias Hauser --- xarray/tests/test_plot.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 0c42ace36c0..894d0ffad38 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -3246,8 +3246,7 @@ def test_plot1d_default_rcparams() -> None: ) # scatter should not emit any warnings when using unfilled markers: - with warnings.catch_warnings(): - warnings.simplefilter("error") + with assert_no_warnings(): fig, ax = plt.subplots(1, 1) ds.plot.scatter(x="A", y="B", ax=ax, marker="x") From affee39aeb0e175fb2d7adaf3828c7155f1d0523 Mon Sep 17 00:00:00 2001 From: Illviljan <14371165+Illviljan@users.noreply.github.com> Date: Tue, 29 Nov 2022 19:05:51 +0100 Subject: [PATCH 12/12] import assert_no_warnings --- xarray/tests/test_plot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 894d0ffad38..9d5039a3df4 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -3,7 +3,6 @@ import contextlib import inspect import math -import warnings from copy import copy from datetime import datetime from typing import Any, Callable, Hashable, Literal @@ -31,6 +30,7 @@ from . import ( assert_array_equal, assert_equal, + assert_no_warnings, requires_cartopy, requires_cftime, requires_matplotlib,