Skip to content

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

Conversation

Kojoley
Copy link
Member

@Kojoley Kojoley commented Oct 5, 2018

The purpose of the patch is to make tracebacks from cache rebuilding clearer.

@Kojoley Kojoley added this to the v3.1 milestone Oct 5, 2018
except json.JSONDecodeError:
_log.warning("Font cache parsing failed %s", _fmcache)

if (not hasattr(fontManager, '_version') or
Copy link
Member

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?

Copy link
Member Author

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

Copy link
Member

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...

Copy link
Member Author

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.
@Kojoley Kojoley force-pushed the move-font-cache-rebuild-out-of-exception-handler branch from 72444e6 to 8d6196e Compare October 5, 2018 22:11
@tacaswell tacaswell merged commit 9869fd7 into matplotlib:master Oct 6, 2018
@tacaswell
Copy link
Member

Thanks @Kojoley !

@Kojoley Kojoley deleted the move-font-cache-rebuild-out-of-exception-handler branch October 6, 2018 11:07
@Kojoley Kojoley mentioned this pull request Oct 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants