Skip to content
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

TST : force re-building of font-cache #3074

Merged
merged 1 commit into from May 19, 2014
Merged

Conversation

tacaswell
Copy link
Member

  • there should be a better way to do this, this will get the
    tests passing again.

as suggested by @efiring in #3073

closes #3065

 - there should be a better way to do this, this will get the
   tests passing again.
@tacaswell tacaswell added this to the v1.4.0 milestone May 19, 2014
@tacaswell
Copy link
Member Author

@efiring @mdboom Can one of you take a look at this soon? I would like to get this merged so travis is useful again.

This snippet illustrates the problem:

from matplotlib import rc_context
from matplotlib.font_manager import findfont, FontProperties, _rebuild
import os

font = findfont(
        FontProperties(family=["sans-serif"]))

with rc_context(rc={
        'font.sans-serif':
        ['cmmi10', 'Bitstream Vera Sans']}):

    font2 = findfont(
        FontProperties(family=["sans-serif"]))

with rc_context(rc={
        'font.sans-serif':
        ['cmmi10', 'Bitstream Vera Sans']}):
    _rebuild()
    font3 = findfont(
        FontProperties(family=["sans-serif"]))

font4 = findfont(
        FontProperties(family=["sans-serif"]))
_rebuild()
font5 = findfont(
        FontProperties(family=["sans-serif"]))

print os.path.basename(font), "should be vera"
print os.path.basename(font2), "should be cmmi"
print os.path.basename(font3), "should be cmmi"
print os.path.basename(font4), "should be vera"
print os.path.basename(font5), "should be vera"

There probably should be a check on RcParams.update to re-build the font-cache if any font-related values are changed, but I don't know that section of the code well enough to see how to do it quickly (I am also worried that doing so will induce circular imports). Long term this should be fixed, short term this change to the test makes it pass (and exercises that the fontmanager works properly) and we should add a warning to the docs that changing font rcparams requires rebuilding the fontmanager.

@mdboom
Copy link
Member

mdboom commented May 19, 2014

Thanks for all the investigation on this -- ideally, changing font params should not require rebuilding of the cache -- the cache should be purely informational. I'll have a look to see if there's a way around that, and this additional information is really helpful in honing in on what might be going wrong.

@mdboom
Copy link
Member

mdboom commented May 19, 2014

@tacaswell: I've restarted the Travis build here (and will probably do it a few more times) just to make sure your test fix does the trick... If it does, that really diagnoses the problem quite well and paves the way forward for a more complete solution. Honestly, I've spent a long time on this, but this is the closest to a diagnosis we've come, so thanks a lot.

efiring added a commit that referenced this pull request May 19, 2014
TST : force re-building of font-cache
@efiring efiring merged commit 4af8e7d into matplotlib:master May 19, 2014
@efiring
Copy link
Member

efiring commented May 19, 2014

I've merged this, as you say, to get tests passing. What is needed as a real fix, though, does not have to be triggering a rebuild of the font manager upon rcParams update, but merely clearing of the lookup caches inside the font manager.

@mdboom
Copy link
Member

mdboom commented May 19, 2014

See #3077 for such a fix. If #3077 proves to be effective, we should roll this back so that the bug is still exercised by the tests.

@tacaswell tacaswell deleted the fp_fix branch May 19, 2014 17:34
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.

font priority bug
3 participants