Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use plt.rc_context for default styles #7318

Merged
merged 19 commits into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ Bug fixes
By `Michael Niklas <https://github.com/headtr1ck>`_.
- Fix static typing of :py:meth:`xr.polyval` (:issue:`7312`, :pull:`7315`).
By `Michael Niklas <https://github.com/headtr1ck>`_.
- Fix matplotlib raising a UserWarning when plotting a scatter plot
with an unfilled marker (:issue:`7313`, :pull:`7318`).
By `Jimmy Westling <https://github.com/illviljan>`_.

Documentation
~~~~~~~~~~~~~
Expand Down
55 changes: 30 additions & 25 deletions xarray/plot/dataarray_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -894,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()

Expand All @@ -905,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)
Expand Down Expand Up @@ -984,29 +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:
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):
Copy link
Contributor

@dcherian dcherian Nov 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this override any user style settings in .matplotlibrc so if they chose a default of black we'd be overwriting that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the innermost context takes priority. edgecolor="k" can be used though.
We have the same problem with color maps:

ds = xr.tutorial.scatter_example_dataset(seed=42)

fig, axs = plt.subplots(1, 2)
with plt.rc_context(
    {
        "scatter.edgecolors": "k",
        "image.cmap": "Greys",
    }
):
    # Normal scatter:
    axs[0].scatter(ds.A.values.ravel(), ds.B.values.ravel(), c=3 * ds.B.values)

    # xarray scatter:
    ds.plot.scatter(x="A", y="B", ax=axs[1])

image

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())
Expand Down Expand Up @@ -1251,9 +1259,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())

Expand Down
37 changes: 37 additions & 0 deletions xarray/tests/test_plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import contextlib
import inspect
import math
import warnings
from copy import copy
from datetime import datetime
from typing import Any, Callable, Hashable, Literal
Expand Down Expand Up @@ -3220,3 +3221,39 @@ 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_plot1d_default_rcparams() -> None:
import matplotlib as mpl

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="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")
)

# scatter should not emit any warnings when using unfilled markers:
with warnings.catch_warnings():
warnings.simplefilter("error")
Illviljan marked this conversation as resolved.
Show resolved Hide resolved
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="A", y="B", marker="o", ax=ax, edgecolor="k")
np.testing.assert_allclose(
ax.collections[0].get_edgecolor(), mpl.colors.to_rgba_array("k")
)