FontManager fixes. #15941
FontManager fixes. #15941
Conversation
|
I don't think this addresses my issue (#13852):
This would need changes somewhere in lines 1288-1299. The actual logic when to rebuild and when not is yet to be clarified. That's why #13852 is "work in progress". |
|
OK, but I think previous versions of rebuilding would not work anyways because the correct fontManager instance was not swapped? |
lib/matplotlib/font_manager.py
Outdated
| _log.debug("Using fontManager instance from %s", fm_path) | ||
| return fm | ||
| fm = FontManager() | ||
| with cbook._lock_path(fm_path): |
There was a problem hiding this comment.
fm_path is only set in the if, so this should not work in your new call with try_read_cache=False. Additionally, you should keep the lock held for the entirety of the function to prevent race conditions. I'm confusing this with #16111, but if that goes through, the lock really should be held for the entirety of this function.
There was a problem hiding this comment.
I'll wait for #16111 to go in first?
|
rebased. |
Previously, when using findfont with rebuild_is_missing (the default) _rebuild() would generate a new fontManager instance, but because findfont is defined as `findfont = fontManager.findfont`, this would keep using the old fontManager instance; we can't just fix this with `def findfont(...): return fontManager.findfont(...)` because people may be importing the fontManager instance anyways and not benefitting from the rebuilt one. Instead, overwrite(!) the contents of the existing fontManager instance with the new one as needed.
|
Travis passed, though it doesn't want to post. |
|
Is there a way we can test this; maybe using a tempdir with a font that gets deleted? |
|
I think that would be rather tricky (or require quite a bit of surgery), if we don't want the test suite to fully regen the font cache every time (you'd need to be able to point the font cache to another path, patch FontManager() to let it temporarily find fonts in a tmpdir, etc.). |
Previously, when using findfont with rebuild_is_missing (the default)
_rebuild() would generate a new fontManager instance, but because
findfont is defined as
findfont = fontManager.findfont, this wouldkeep using the old fontManager instance; we can't just fix this with
def findfont(...): return fontManager.findfont(...)because people maybe importing the fontManager instance anyways and not benefitting from
the rebuilt one.
Instead, overwrite(!) the contents of the existing fontManager instance
with the new one as needed.
@timhoffm IIRC you had some issues with font cache rebuilding, perhaps this helps?
PR Summary
PR Checklist