Skip to content

Use json for the font cache instead of pickle#5021

Merged
mdboom merged 2 commits into
matplotlib:masterfrom
jkseppan:json-font-cache
Oct 6, 2015
Merged

Use json for the font cache instead of pickle#5021
mdboom merged 2 commits into
matplotlib:masterfrom
jkseppan:json-font-cache

Conversation

@jkseppan

@jkseppan jkseppan commented Sep 3, 2015

Copy link
Copy Markdown
Member

See discussion at #4756.

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

Copy link
Copy Markdown
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

mdboom commented Sep 3, 2015

Copy link
Copy Markdown
Member

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

jkseppan commented Sep 3, 2015

Copy link
Copy Markdown
Member Author

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

mdboom commented Sep 3, 2015

Copy link
Copy Markdown
Member

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

mdboom commented Oct 6, 2015

Copy link
Copy Markdown
Member

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

@jkseppan

jkseppan commented Oct 6, 2015

Copy link
Copy Markdown
Member Author

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

mdboom commented Oct 6, 2015

Copy link
Copy Markdown
Member

Yes -- definitely not for 1.5.

@tacaswell

Copy link
Copy Markdown
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