-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Be less tolerant of broken installs. #12246
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
Conversation
If no user-defined matplotlibrc is present, require it to exists in mpl-data. (We already throw when mpl-data cannot be found -- which is also a sign of broken install -- so it's not much worse to require matplotlibrc to exist as well.)
70bea2e
to
7ef113b
Compare
Can you comment on why it would be necessary for some rc file to exist? It's usually a completely commented out file, so it serves no purpose, and does no harm if it's not found. |
Because the build script creates it (at https://github.com/matplotlib/matplotlib/blob/master/setup.py#L204). |
I'd say a user-friendly software would try to repair itself if it finds itself in a currupted state. I mean it's not obvious from anywhere that this file needs to exist. |
You also really need to mess it up/do it on purpose to have an install where the file doesn't exist (it's like an install where axes/_base.py doesn't exist, sure you could have deleted it accidentally but other than that...). |
So what happens if I nuke ~/.matplotlib ? Does it get recreated by init? |
We already look in $prefix/matplotlib/mpl-data/matplotlibrc where $prefix/matplotlib is the place matplotlib is actually installed. And that file is created at install time, in fact it's the one that gets picked up when you haven't configured your own matplotlibrc. |
I am not sure we want to impose this on our down-stream packagers. We know that various linux distributions significantly de-construct our installation. attn @sandrotosi . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyone can dismiss this, I just want to make sure that we check with down-stream packagers that this will not cause them headaches.
In fact, I ran into issues with the fallback when the rc file did not exist. We used to have an rc file that just specified the backend, but dropped that since mpl can pick one automatically now. Since we patch the system location to be in Whatever happens when an rc file (even if the rc file just specifies a backend and nothing else) is loaded didn't happen with the fallback, and a lot of params were their unvalidated non-canonical forms that didn't work when actually used. TLDR, we already patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me.
I am going to merge this as is, if we find it to be a problem, we should add a fallback of reading an empty file from a temp location. |
sorry for the late reply, but this is fine even for Debian - we force |
If no user-defined matplotlibrc is present, require it to exists in
mpl-data. (We already throw when mpl-data cannot be found -- which is
also a sign of broken install -- so it's not much worse to require
matplotlibrc to exist as well.)
PR Summary
PR Checklist