Load style files from third-party packages. #24257
Conversation
8a7bb67 to
4a815ae
Compare
| # The approach of using ``mpl_configdir`` should be considered legacy, as it | ||
| # does not allow distribution on PyPI. |
There was a problem hiding this comment.
Do we want to go this far? I think I prefer the method you are advocating here, but the old method is surely not going away at any point is it?
There was a problem hiding this comment.
It's unlikely to go away for years, but I think it's still good to be opinionated if we think one approach is clearly better (I think mplconfigdir is clearly worse...).
There was a problem hiding this comment.
I think it depends. For installing someone else's style file, I agree pip install cool-style-from-internet is preferable. But if I want to write my own style file and stick it somewhere global, mpl_configdir is easier than writing my own package and pip-installing it. I could figure out how to do it, and I'd probably go through the bother because I change machines enough, and its more reproducible because I can make it an environment dependency. But, a lot of our end users aren't going to know how to make their own pip-installable package that contains a style sheet.
There was a problem hiding this comment.
Sure, let's just remove that for now.
|
Unlike style files, which exist next to modules, colormaps currently have no representation other than as python objects living in a module, so I don't think this is easily feasible. |
There was some early talk of developing a spec for specifying color maps (basically enough metadata to give a name + classification information and then a CSV of RGB values), but that did not come to fruition. |
|
I am 👍🏻 on this, just have a minor nit code wise. |
There was a problem hiding this comment.
This is great. It's simple, it solves the distribution problem and it has namespacing (and thus avoids name conflicts).
I had the idea whether we would want to support a mypackage/default.mplstyle so that one can do mpl.style.use('mypackage'). This could be nicer than forcing packages to name their style. One could have mpl.style.use('seaborn') instead of mpl.style.use('seaborn.seaborn') or other similar constructs.
| as follows. Assume that a package is importable as ``import my.package``, with | ||
| a ``my/package/__init__.py`` module. Then a | ||
| ``my/package/presentation.mplstyle`` style sheet can be used as | ||
| ``plt.style.use("my.package.presentation")``. (Note that the implementation | ||
| does not actually import ``my.package``, making this process safe against |
There was a problem hiding this comment.
| as follows. Assume that a package is importable as ``import my.package``, with | |
| a ``my/package/__init__.py`` module. Then a | |
| ``my/package/presentation.mplstyle`` style sheet can be used as | |
| ``plt.style.use("my.package.presentation")``. (Note that the implementation | |
| does not actually import ``my.package``, making this process safe against | |
| as follows. Assume that a package is importable as ``import mypackage``, with | |
| a ``mypackage/__init__.py`` module. Then a | |
| ``mypackage/presentation.mplstyle`` style sheet can be used as | |
| ``plt.style.use("mypackage.presentation")``. (Note that the implementation | |
| does not actually import ``mypackage``, making this process safe against |
Let's keep this top-level. Even if a package has subpackages, there should almost never be a reason to hide the styles in lower levels.
You may mention the edge-case in yet to be written documentation.
My general idea is that most packages will want to distribute more than one style file anyways, so e.g. I've handled your other comments. |
d3046d8 to
01c8743
Compare
|
I have minor concerns on ambiguity of strings builtin / style file / package style. We should either think this through or make it provisional so that we don't render us in a corner unintendedly. |
|
(made the example in customizing.py show a non-dotted package name first as well). I'm totally fine with making this provisional. |
... and use the shorter _rc_params_in_file.
|
thanks, fixed both (kept a mention about subpackages but it comes at the end now). |
| @@ -334,6 +334,11 @@ def make_release_tree(self, base_dir, files): | |||
| os.environ.get("CIBUILDWHEEL", "0") != "1" | |||
| ) else [] | |||
| ), | |||
| extras_require={ | |||
There was a problem hiding this comment.
I think this is not doing what you would expect it to.
Python version specific markers can be achieved with
importlib-resources>=3.2.0; python_version<"3.10"
This should be in the main dependencies section, not as an extra.
There was a problem hiding this comment.
This does actually work, I checked when I had the same thought previously. It is a bit outdated, and what you linked is the more recommended way, but this is valid (if perhaps not fully future-proof, as it is not well documented any more).
PR Summary
The first commit inlines the logic of the undocumented _remove_blacklisted_style_params and _apply_style helpers into mpl.style.use, which makes it easier to follow (the exact semantics of
_apply_styleare really not that clear without going to check its implementation anyways). It's mostly just preparation for the second commit.The second commit implements loading of style files from third-party packages (as
mpl.style.use("package.name.style_name"), which loadspackage/name/style_name.mplstyle). This fixes the long-standing issue of how third-parties can distribute style sheets (for those who really don't want to actually import the third-party package).The exact syntax and semantics could be discussed, but I think the basic design is there.
Fixes #17978
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).