-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[Bug]: 3.5.0rc1: MPLCONFIGDIR no longer supported to load custom configurations? #21375
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
Comments
Can you please provide a simple repro for devs not running debian? (e.g. using docker, or generic unix commands, etc.) |
i can try:
and finally, at the end you should get the error:
the debian build system is pretty crazy for mpl, let me know if you want me to explain some of its parts in details but this should get you going. Just to cover all our bases, we do have some patches we apply https://salsa.debian.org/python-team/packages/matplotlib/-/tree/master/debian/patches and while preparing for this reply, i noticed that not applying those patches (so skipping the |
Unfortunately, the above results in
While we're at it, can you modify the above so that the docs can be built with |
I was able to replicate the error, but only when partially applying https://salsa.debian.org/python-team/packages/matplotlib/-/blob/master/debian/patches/20_matplotlibrc_path_search_fix.patch (ie either one of the 2 hunks without the other), but i was also able to get rid of the error reported initially by NOT applying that patch, which puzzles me: as i said before, we've been using variations of this patch for a long time |
oh apologies, i did not properly read the error: i think you need to run now the sequence of commands should be
with no patches applied, this fails with
which is expected, since sphinx_panels is not available in debian (yet, guess i'm gonna have to package it) |
I've also tried to apply @QuLogic 's patch from fedora-python@4039d6d, but it fails the same way:
so, the only reason it works without patch it's because matplotlib is reading the file available in the source dir:
note the
I think this should show that MPLCONFIGDIR (or MATPLOTLIBRC) is no longer functional, because the command line clearly has |
What does this mean? Note, as of #19603, that file is the source of rcParam defaults. So you should be wary of changing it, and it should not, e.g., be moved to |
Thanks, i think that clarified what's happening: everytime In Debian, we dont ship mpl-data along with the module, but we split it in a separate package, and change the path to look for that file with this patch, but the patch is applied before the module is built (setup.py build), and so it's included in the installed module and used when running tests or building doc. What complicates things even more, is that some of the packages we need to build matplotlib depends on matplotlib itself so there the old version of mpl installed on the same system where we're building the new version. So our patch lets mpl find the old matplotlibrc file, which does not contain new entries (like Im open to suggestions, the first thought i have is to change this to do something like:
the first branch will only work during build (which is unfortunate, because it means it's a test that fail for all installations, but oh well) and the second branch will be trigger when you install the package from the apt repo what you think? |
I assume the circular dependency is from the docs? I wonder if the easiest path is for the debian packaging to move the diff --git a/lib/matplotlib/__init__.py b/lib/matplotlib/__init__.py
index 6f146b7197..c606559b9b 100644
--- a/lib/matplotlib/__init__.py
+++ b/lib/matplotlib/__init__.py
@@ -863,7 +863,7 @@ Please do not ask for support with these customizations active.
# by explicitly calling the superclass (dict.update, dict.items) to avoid
# triggering resolution of _auto_backend_sentinel.
rcParamsDefault = _rc_params_in_file(
- cbook._get_data_path("matplotlibrc"),
+ "/usr/share/matplotlib/matplotlibrc",
# Strip leading comment.
transform=lambda line: line[1:] if line.startswith("#") else line,
fail_on_error=True)
and leave the rest of the mpl-data related patches as-is? |
matplotlib/lib/matplotlib/cbook/__init__.py Line 512 in 9568e14
/usr/share/matplotlib/mpl-data so i'm not sure this would work
i think i need a way to differentiate between build-time and run-time, previously i'd just stick a matplotlibrc file crafted specifically for build in the right place and use MPLCONFIGDIR to load it up, but with the removal if the default rc params in 178515c and #19603 that no longer works |
That is why the Fedora patch uses a relative path for |
yeah i tried to use that, but sadly it wont work, because parent.parent.parent... for a short path (like |
but you are packaging mpl-data separately so it may no be there? By dropping the |
Why does that matter; it's the source path. Does Debian not install to a build root that mirrors the final location? |
the problem here is that the same code needs to support 2 different situations:
i think this will work:
at least the build passes the previous error (i cannot test the second point until i manage to generate the final packages). Now the problem seems to be with latex? (if you prefer me to open a new bug or retitle this one in a generic "support debian packaging of 3.5.0rc1" lemme know).
any idea what could possibly go wrong now? i found old reports suggesting to install packages that are already available in the build environment or setting rcParams that are no longer valid. thanks! |
As a meta comment, how optional is If we don't consider If we do consider it optional, I dont' think we can continue to have the canonical I'm not sure how optional we consider the fallback fonts. They seem pretty non-optional, though I can imagine a good packager removing them if they know they will not be needed. |
I can't find it right now, but that's a known bug of tildes in the path ( |
this would be my preference. |
I don't think any of mpl-data should be considered as optional. If debian wants to dedupe the files, I believe the onus is on their side to replace the files e.g. by symlinks, so that the contents effectively stay the same. |
thanks! the fix in #21414 worked (now there are other underlying issues outside of mpl that prevents to complete the packaging which i'm going to work on) |
@sandrotosi We discussed this on the weekly call, just to make sure we all agreed on the messaging here. For quite a while It sounds like you already have this largely sorted to your satisfaction, but we just wanted to clarify how we currently view and use this directory. @tacaswell, @QuLogic and @anntzer can clarify any of the above if I have it wrong or have missed a subtlety (I just use conda on a Mac 😉 ). |
Bug summary
Hello,
while packaging 3.5.0rc1, i notice that it's no longer possible to set a custom
matplotlibrc
, if the default one is "invalid".(that's because
/usr/share/matplotlib/mpl-data/matplotlibrc
is a template file in Debian's setup); the same exact error happens even settingMATPLOTLIBRC=./matplotlibrc
, as specified in the doc the doc).This used to work, and it's the way how we set specific settings for our build environment (like backend, etc).
looking at the code in
__init__.py
it appears_rc_params_in_file()
is always executed against whatcbook._get_data_path("matplotlibrc")
returns, which with a couple of hoops results in a static file (mind that we customize this location in debian as seen here, which is the path you see above ).Without the possibility to either use MPLCONFIGDIR or MATPLOTLIBRC, building matplotlib in Debian becomes quite hard. Is this just an oversight and support should be re-introduced, or is this a conscious decision?
thanks,
Sandro
Code for reproduction
Actual outcome
traceback
Expected outcome
no traceback
Operating system
Debian
Matplotlib Version
3.5.0rc1
Matplotlib Backend
No response
Python version
3.9.7
Jupyter version
No response
Other libraries
No response
Installation
source
Conda channel
No response
The text was updated successfully, but these errors were encountered: