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

Fix font family lookup calculation #2771

Merged
merged 4 commits into from Apr 28, 2014

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Jan 27, 2014

As reported in mailing list thread "font setting in matplotlib 1.3.1"

['cmmi10', 'Bitstream Vera Sans']}):
font = findfont(
FontProperties(family=["sans-serif"]))
assert os.path.basename(font) == 'cmmi10.ttf'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file going to be available on all distros?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It ships with matplotlib and is required for mathtext support (that's why I chose it). Some distros still monkey around with not including required fonts, of course.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should mention that using assert_equal is much nicer, as when it fails, it has enough context to tell you why they are different.

@pelson
Copy link
Member

pelson commented Jan 28, 2014

The test failiure is genuine on 2.6:

======================================================================
FAIL: matplotlib.tests.test_font_manager.test_font_priority
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/matplotlib-1.4.x-py2.6-linux-x86_64.egg/matplotlib/tests/test_font_manager.py", line 18, in test_font_priority
    assert os.path.basename(font) == 'cmmi10.ttf'
AssertionError```

@mdboom
Copy link
Member Author

mdboom commented Jan 28, 2014

The test failure is random. It necessarily changes global state, so when running the tests in parallel, there are problems. I think I need to look into how to "lock" this test so it doesn't run in parallel.

@pelson
Copy link
Member

pelson commented Jan 28, 2014

It necessarily changes global state, so when running the tests in parallel, there are problems.

I'm very surprised - surely these tests are being run in multiple processes rather than multiple threads, so surely we're not impacted by changing globals? I'm sure I've missed something here though...

@@ -1071,7 +1071,7 @@ def score_family(self, families, family2):
if family2 in options:
idx = options.index(family2)
return ((0.1 * (float(idx) / len(options))) *
(float(i) / float(len(families))))
(float(i + 1) / float(len(families))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't `from future_import division remove the need for all of this casting to float?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. I think this code was written prior to that. Will fix.

list (e.g. font.sans-serif) would be considered a perfect match whenever
the selected font alias is the first in the font.family list.  This
broke the priority ordering of font names.
@tacaswell
Copy link
Member

3.2 (as normal it seems) still has issues.

@mdboom
Copy link
Member Author

mdboom commented Feb 26, 2014

Frustratingly, I can't reproduce it locally -- seems to be Travis-specific.

@tacaswell tacaswell modified the milestones: v1.4.0, v1.3.x Apr 17, 2014
tacaswell added a commit that referenced this pull request Apr 28, 2014
@tacaswell tacaswell merged commit 063e60f into matplotlib:master Apr 28, 2014
@tacaswell
Copy link
Member

@mdboom It seems to have passed last time it ran, I am going to merge it.

@mdboom mdboom deleted the fonts/fix-lookup-mechanism branch August 7, 2014 13:54
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.

None yet

5 participants