-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Deprecate the TTFPATH & AFMPATH environment variables. #15943
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
Thanks for letting me know -- works for me! I'll add a patch to |
Actually, just realized this might be a significant slowdown since proplot's fonts (and indeed all user-added fonts) would no longer be cached. I see a couple options here:
Would you consider adding one of these to the branch? I am partial to # 2 :) |
But dumping is actually already public API (json_dump)? (Well, sure, perhaps we can move _lock_path into json_dump.) So you can just add your fonts and call json_dump? |
Ok, makes sense. Yeah my reason for suggesting the |
Yup, I wrote #16111 hoping this would help your use case :) |
It does help, thanks! |
fdc5044
to
52e3125
Compare
Maybe instead store |
This doesn't really help -- this would only be useful if the envvars are set before importing mpl anyways, as we can't (and don't want to...) register a listener for changes in the variables (rebuilding is private API), but it's really hard for anyone to be sure matplotlib wasn't imported before them. Also I think addfont() provides more than enough functionality to replace them. |
That's the purpose of environment variables, is not it?
It does not look like it is, as @lukelbd has to rewrite the cache file manually, what is a hack in my opinion, and #16111 just makes the hack smaller. Probably we are missing something. Why @lukelbd wants to add fonts at runtime, while at the same time needs them to be in the cache at startup? |
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.
As of PR itself, it is indeed better to deprecate those envvars if a cache is not regenerated according to them.
TTFPATH and AFMPATH were not sufficient previously anyways, @lukelbd needed additionally to call the private _rebuild() to make them work (as he will always need when using environment variables), which means that technically we're completely within our rights to break that. |
It depends on a purpose. Like if something Conda-like has it's own font directory and that virtual environment activation changes |
Fair enough, but that's clearly an terrible and brittle API (needing to set a bunch of env vars for something that should just be a simple API call; moreover this doesn't even pick up new added fonts). |
That's already an issue of the current font manager on OSX/Windows too, is not it? |
Yes, and it should be fixed -- but in a sense there's no API for users to add fonts; it seems worse to have an API which is "you can do this to add fonts to the path -- unless someone already did it some time ago in which case it won't work". |
These undocumented environment variables allow one to selectively add paths to the font cache... but only if the font cache doesn't exist yet (they don't force cache regen). The font-cache regen API (_rebuild()) is also private (even though one can trigger it "publically" e.g. by calling `findfont` with a nonexisting font...). Instead, we now expose `addfont()` which is a documented API for registering fonts with the font manager.
These undocumented environment variables allow one to selectively add
paths to the font cache... but only if the font cache doesn't exist yet
(they don't force cache regen). The font-cache regen API (_rebuild())
is also private (even though one can trigger it "publically" e.g. by
calling
findfont
with a nonexisting font...).Instead, we now expose
addfont()
which is a documented API forregistering fonts with the font manager.
attn @lukelbd Your proplot package is the only one I am aware of that uses TTFPATH; how does addfont() look to you?
PR Summary
PR Checklist