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

Conversation

Illviljan
Copy link
Contributor

@Illviljan Illviljan commented Nov 23, 2022

@Illviljan
Copy link
Contributor Author

Illviljan commented Nov 23, 2022

Seaborn did something similar as the original but had a little check if the marker was filled or not:
https://github.com/mwaskom/seaborn/blob/bf4695466d742f301f361b8d0c8168c5c4bdf889/seaborn/relational.py#L563

@Illviljan Illviljan marked this pull request as ready for review November 26, 2022 15:55
@Illviljan Illviljan changed the title Use rc context for default styles Use plt.rc_context for default styles Nov 26, 2022
Copy link
Collaborator

@headtr1ck headtr1ck left a comment

Choose a reason for hiding this comment

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

Did not check what seaborn is doing exactly, but it seems like a reasonable approach.

@Illviljan Illviljan added the plan to merge Final call for comments label Nov 27, 2022
xarray/tests/test_plot.py Outdated Show resolved Hide resolved
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

@Illviljan
Copy link
Contributor Author

This has stalled for long enough. I'll merge this at the end of next week unless someone disagrees, at least it is an improvement to the bugged main.

@Illviljan Illviljan merged commit 7683442 into pydata:main Feb 9, 2023
dcherian added a commit to bzah/xarray that referenced this pull request Feb 28, 2023
* upstream/main: (291 commits)
  Update error message when saving multiindex (pydata#7475)
  DOC: cross ref the groupby tutorial (pydata#7555)
  [pre-commit.ci] pre-commit autoupdate (pydata#7543)
  supress namespace_package deprecation warning (doctests) (pydata#7548)
  [skip-ci] Add PDF of Xarray logo (pydata#7530)
  Support complex arrays in xr.corr (pydata#7392)
  clarification for thresh arg of dataset.dropna() (pydata#7481)
  [pre-commit.ci] pre-commit autoupdate (pydata#7524)
  Require to explicitly defining optional dimensions such as hue and markersize (pydata#7277)
  Use plt.rc_context for default styles (pydata#7318)
  Update HOW_TO_RELEASE.md (pydata#7512)
  Update whats-new for dev (pydata#7511)
  Fix whats-new for 2023.02.0 (pydata#7506)
  Update apply_ufunc output_sizes error message (pydata#7509)
  Zarr: drop "source" and  "original_shape" from encoding (pydata#7500)
  [pre-commit.ci] pre-commit autoupdate (pydata#7507)
  Whats-new for 2023.03.0
  Add `inclusive` argument to `cftime_range` and `date_range` and deprecate `closed` argument (pydata#7373)
  Make text match code example (pydata#7499)
  Remove Dask single-threaded setting in tests (pydata#7489)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using scatter with unfilled marker raises matplotlib UserWarning
5 participants