-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Get default params from matplotlibrc.template. #14929
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
326ec0e
to
6f62844
Compare
Some questions / notes:
Semi-OT: I think the config system could need a general overhaul, with among other things a simple way of resetting to defaults, layering, a public interface to add params for downstream libs, inferred values, etc. I've started a bit thinking about these things, but it's a bit early to come up with a proposal. This would also address the duplicate defaults. |
I think that's approximately the same as requiring
Yes, it will fail in
(
It basically makes no sense for the end user to change it; see discussion at #11241. I plan to deprecate it.
AFAICT This PR does not break the public API; probably you can still even append rcParams and have things work, not that I would recommend that...
Indeed, see https://github.com/matplotlib/mplcairo#antialiasing for an example case. |
So now most users will have everything uncommented in their Matplotlibrc? That seems difficult to manage. How will a user easily see when they have made a non default change? Or when you install the template for a user who doesn’t have one does it replace all the comment marks? |
6da81ee
to
7888374
Compare
Users should not modify the one that's in matplotlib/mpl-data (which matplotlibrc explicitly warns against anyways), they should just place their own matplotlibrc in ~/.config/matplotlib or ~/.matplotlib anyways (which we don't create upon install), and that file can just have whatever entries they need. |
Mine is definitely from the template. Though you are correct that it wasn't put there. But the way I originally made it was to copy the old template and search for the key I wanted and uncommented it and changed it. This change would make that harder. |
This makes matplotlibrc.template a valid style file representing the default style (modulo things that styles do not control, such as the backend), avoids having to duplicate the defaults in rcsetup, removes the need to test that the default matplotlibrc is up to date (if not, matplotlib fails to import), and makes it easier to fit lines within 79 characters in rcsetup.py. The only tricky points are: - datapath is not really an rcparam anyways, so just hack `__getitem__` to always return the correct value. - the entry for path.effects was incorrect, as [] would be validated as the "[]" list which is not a valid list of patheffects. In fact this rcParam cannot be meaningfully set from matplotlibrc; one would need to do an eval() like for axes.prop_cycle but let's not get there... I changed the validator to validate_anylist, which at least works for the default of an empty list... - we need to be a bit more careful when constructing the global rcParams instance as well as rcParamsOrig, rcParamsDefault, to not resolve _auto_backend_sentinel too early. One can check that the rcParams are unchanged by printing them out -- that catches a typo: two entries in font.fantasy should be "Impact", "Western"; not "ImpactWestern".
I would say that if you care about tracking which keys you've changed away from the default, it's easier to create a new file and just copy the lines that you want to change to the new file, rather than copying a file with hundreds of lines and uncommenting a few of them? |
I fear a bit that this is what would happen... People will copy that file, make the change they want to make and have all the other parameters activated in their customized rc file. This can lead to all kinds of problems, like style files not being compatible between versions, keeping deprecated parameters around or absolute paths to video converters being distributed as style files. My proposal would be as follows: If you want to have the parameters loaded from a dedicated file, it should not be the |
Yeah I think people will definitely copy the full file. For all they know there is some magic in there somewhere that needs to be in the file so caution says to copy the whole file. |
Could also do it the other way round: Keep |
I guess I don't actually have a good guess as to where people are copying the full file from: from the repo's matplotlibrc.template? from https://matplotlib.org/tutorials/introductory/customizing.html#a-sample-matplotlibrc-file? from their install's site-packages/matplotlib/mpl-data/matplotlibrc? Because this affects where we should put the default style that gets picked up on import. |
That seems reasonable. Only thing bad there is that someone will edit that file in place at some point, but not sure what we can do about that. |
OK, see alternate implementation at #15029. |
I prefer #15029 to this one. |
Closing in favor of #15029. |
This makes matplotlibrc.template a valid style file representing the
default style (modulo things that styles do not control, such as the
backend), avoids having to duplicate the defaults in rcsetup, removes
the need to test that the default matplotlibrc is up to date (if not,
matplotlib fails to import), and makes it easier to fit lines within 79
characters in rcsetup.py.
The only tricky points are:
__getitem__
to always return the correct value.
the "[]" list which is not a valid list of patheffects. In fact
this rcParam cannot be meaningfully set from matplotlibrc; one would
need to do an eval() like for axes.prop_cycle but let's not get
there... I changed the validator to validate_anylist, which at least
works for the default of an empty list...
instance as well as rcParamsOrig, rcParamsDefault, to not resolve
_auto_backend_sentinel too early.
One can check that the rcParams are unchanged by printing them out --
that catches a typo: two entries in font.fantasy should be "Impact",
"Western"; not "ImpactWestern".
PR Summary
PR Checklist