-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Move font cache rebuild out of exception handler #12416
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
Move font cache rebuild out of exception handler #12416
Conversation
lib/matplotlib/font_manager.py
Outdated
except json.JSONDecodeError: | ||
_log.warning("Font cache parsing failed %s", _fmcache) | ||
|
||
if (not hasattr(fontManager, '_version') or |
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.
can't fontManager
be None
here? If so, won't fontManager._version != FontManager.__version__
give an AttributeError?
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.
>python -c "fontManager = None; print(not hasattr(fontManager, '_version') or fontManager._version != 123)"
True
However (version mismatch)
log message is misleading in this situation
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.
Python is so unexpected sometimes...
I guess the log message is misleading, but you'll already have had one before with the real error, so prob fine? Or you could just check that fontManager is not None before the rest of the conditional...
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.
I have updated the PR, now we should be fine.
The purpose of the patch is to make tracebacks from cache rebuilding clearer.
72444e6
to
8d6196e
Compare
Thanks @Kojoley ! |
The purpose of the patch is to make tracebacks from cache rebuilding clearer.