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

ENH: add reusable matplotlib stylesheet #4343

Merged
merged 6 commits into from
Mar 22, 2023

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented Feb 18, 2023

PR Summary

Adresses #4342, but doesn't close it because I'm holding off on automated testing until we get pytest_mpl started.

Here are my manual tests for it

import matplotlib as mpl
import matplotlib.pyplot as plt
import yt

from packaging.version import Version
from yt.visualization._commons import MPL_VERSION

print(f'{MPL_VERSION=}')


with yt.funcs.matplotlib_style_context():
    fig, ax = plt.subplots()
    ax.plot([0, 1], [0, 1])
    ax.set(xlabel=r"$\Sigma$", ylabel=r"$\beta$", title="My fake yt plot")
    fig.savefig("/tmp/yt_style_funcs.png")

if MPL_VERSION >= Version("3.7"):
    with plt.style.context("yt.default"):
        fig, ax = plt.subplots()
        ax.plot([0, 1], [0, 1])
        ax.set(xlabel=r"$\Sigma$", ylabel=r"$\beta$", title="My fake yt plot")
        fig.savefig("/tmp/yt_style_plt_context.png")

    with mpl.style.context("yt.default"):
        fig, ax = plt.subplots()
        ax.plot([0, 1], [0, 1])
        ax.set(xlabel=r"$\Sigma$", ylabel=r"$\beta$", title="My fake yt plot")
        fig.savefig("/tmp/yt_style_mpl_context.png")

    mpl.style.use("yt.default")
    fig, ax = plt.subplots()
    ax.plot([0, 1], [0, 1])
    ax.set(xlabel=r"$\Sigma$", ylabel=r"$\beta$", title="My fake yt plot")
    fig.savefig("/tmp/yt_style_mpl_use.png")

I still plan on adding a minimal test with no actual image comparison, just to make sure this works with minimal requirements too (which i have checked manually).
I will also need to add documentation.

PR Checklist

  • New features are documented, with docstrings and narrative docs
  • [N/A] Adds a test for any bugs fixed. Adds tests for new features.
  • make sure the new file is correctly bundled in builds

@neutrinoceros neutrinoceros marked this pull request as draft February 18, 2023 15:46
@neutrinoceros neutrinoceros added bug enhancement Making something better dependencies Pull requests that update a dependency file viz: 2D labels Feb 18, 2023
@neutrinoceros neutrinoceros force-pushed the reusable_stylesheet branch 3 times, most recently from 51dd2ec to 44422c2 Compare February 18, 2023 17:44
@neutrinoceros
Copy link
Member Author

neutrinoceros commented Feb 18, 2023

I may have missed the point of matplotlib_style_context's default behaviour. I assumed it was meant to reproduce yt's style, when it may be a simple backport of matplotlib.style.context for older versions of matplotlib (in which case it's not needed now).
I will focus on getting CI green for now, and I'll come back to this part.

update (answering my own concerns): There may have been a couple competing ideas in the original design of the function, but I'm going to go with my original assumption that it's meant to reproduce yt's style, to some extent, because it is used internally without any arguments.

@neutrinoceros neutrinoceros force-pushed the reusable_stylesheet branch 2 times, most recently from 83d7b91 to f6558cc Compare February 18, 2023 22:24
@neutrinoceros
Copy link
Member Author

Actually, turns out this is already minimally tested since matplotlib_style_context is used internally.

font.family: stixgeneral

mathtext.fontset: cm
mathtext.fallback: cm
Copy link
Member Author

@neutrinoceros neutrinoceros Feb 19, 2023

Choose a reason for hiding this comment

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

hint to reviewers: this option is the reason why I'm dropping support for matplotlib 3.2 in the same PR (it's invalid before matplotlib 3.3). Previously this was handled dynamically but this behaviour cannot be replicated with a static style sheet.

@neutrinoceros neutrinoceros marked this pull request as ready for review February 19, 2023 13:27
@neutrinoceros
Copy link
Member Author

For reference here's the current output of the example I'm giving in the docs:
YTSTYLE

Does it feels like yt enough ? Am I missing some of the parameters that make up for yt's style ?

@matthewturk
Copy link
Member

I do like this idea! Will go over in detail.

@neutrinoceros neutrinoceros added the new feature Something fun and new! label Mar 22, 2023
matthewturk
matthewturk previously approved these changes Mar 22, 2023
Copy link
Member

@matthewturk matthewturk left a comment

Choose a reason for hiding this comment

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

Sorry for letting this languish! I think it's good to go in, don't you?

@neutrinoceros
Copy link
Member Author

I think I should revisit this to avoid data duplication in yt.visualization._commons.DEFAULT_FONT_PROPERTIES, and simplify how it's constructed by using matplotlib API if possible. Switching to draft for now

@neutrinoceros
Copy link
Member Author

Turns out the nice API I was hoping for doesn't exist (yet ?) so I'll just push one more time to fix type-checking, and now I believe it's ready to go @matthewturk

@neutrinoceros neutrinoceros marked this pull request as ready for review March 22, 2023 12:45
@neutrinoceros
Copy link
Member Author

Well looking back, I actually ended up removing the one parameter that justified dropping support for MPL 3.2, so I'll revert this now.

@neutrinoceros neutrinoceros removed the dependencies Pull requests that update a dependency file label Mar 22, 2023
@matthewturk matthewturk enabled auto-merge March 22, 2023 17:13
@matthewturk matthewturk merged commit def170f into yt-project:main Mar 22, 2023
@neutrinoceros neutrinoceros deleted the reusable_stylesheet branch March 22, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug docs enhancement Making something better new feature Something fun and new! viz: 2D
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants