Skip to content

Use json for the font cache instead of pickle #5021

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

Merged
merged 2 commits into from
Oct 6, 2015

Conversation

jkseppan
Copy link
Member

@jkseppan jkseppan commented Sep 3, 2015

See discussion at #4756.

@tacaswell tacaswell added this to the proposed next point release milestone Sep 3, 2015
@WeatherGod
Copy link
Member

Do we even have any unit tests for font caching? The problem with Travis here is that the font cache isn't used there at all, I don't think.

@mdboom
Copy link
Member

mdboom commented Sep 3, 2015

The font cache is always used to draw text, and the test suite draws text. On Travis, the font cache is rebuilt from scratch each time. What we don't have is a test of saving to disk and loading it again. But that should be easy enough to write by calling the various functions in font_manager.py directly from a unit test.

@jkseppan
Copy link
Member Author

jkseppan commented Sep 3, 2015

One problem with testing this class is that it searches the filesystem when you call __init__. But I guess we can work around that.

@mdboom
Copy link
Member

mdboom commented Sep 3, 2015

I think it's probably good enough to just save and reload the existing singleton (font_manager.py:fontManager) to a temp file. That shouldn't do a lot of file I/O (other than the tempfile). The generation of the cache itself from the font files on the system is already tested (or at least covered) by the existing unit test suite.

@mdboom
Copy link
Member

mdboom commented Oct 6, 2015

@jkseppan: Do you consider this ready to merge? Looks good to me.

@jkseppan
Copy link
Member Author

jkseppan commented Oct 6, 2015

I think it should be ready to merge to master, but it's probably too large of a change to add to 1.5.x in the release-candidate phase.

@mdboom
Copy link
Member

mdboom commented Oct 6, 2015

Yes -- definitely not for 1.5.

@tacaswell
Copy link
Member

Master is fair game for 2.1 code.

mdboom added a commit that referenced this pull request Oct 6, 2015
Use json for the font cache instead of pickle
@mdboom mdboom merged commit fef412e into matplotlib:master Oct 6, 2015
@jkseppan jkseppan deleted the json-font-cache branch October 8, 2015 15:22
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.

4 participants