FIX: KeyError while parsing ttfFontProperty by JCorson · Pull Request #365 · enthought/enable

JCorson · 2019-09-22T02:30:52Z

I think this might help the KeyErrors that were exposed when logging was introduced in ttfFontProperty. It continues searching through the names until one can be decoded with either utf-8 or utf-16-be. Rather than using the platformID, platEncID, langID, and nameID, this only uses the nameID. It also removes the sfnt2 prop. AFAICT, it wasn't actually needed. style = 'normal' would be set regardless.

I would prefer to get a font in the repo to test on that was producing the KeyError, but testing locally with createFontList(findSystemFonts()) on OSX, Windows, and Ubuntu ends up with all fonts parsed without raising an exception.

Let me know what you think.

Closes #363

TST: Add test font file from matplotlib

FIX: Correct test method name

FIX: Add missing imports

FIX: Remove __main__ block

rahulporuri · 2019-09-24T09:14:55Z

the travis job for python 2.7 and null toolkit failed with the following traceback -

======================================================================

ERROR: test_text (kiva.tests.test_gl_drawing.TestGLDrawing)

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/tests/drawing_tester.py", line 72, in test_text

    self.gc.show_text("hello kiva")

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/gl.py", line 343, in show_text

    return self.show_text_at_point(text, *point)

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/gl.py", line 349, in show_text_at_point

    pyglet_font = GetFont(self._current_font)

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/gl.py", line 277, in GetFont

    pyglet_font = load_font(font.findfontname(), font.size, bold,

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/fonttools/font.py", line 115, in findfontname

    return fp.get_name()

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/fonttools/font_manager.py", line 878, in get_name

    return getPropDict(TTFont(str(font))['name'])

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/fonttools/font_manager.py", line 246, in getPropDict

    n = font['name']

TypeError: 'table__n_a_m_e' object has no attribute '__getitem__'

-------------------- >> begin captured logging << --------------------

fontTools.ttLib.ttFont: DEBUG: Reading 'name' table from disk

fontTools.ttLib.ttFont: DEBUG: Decompiling 'name' table

--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------

JCorson · 2019-09-24T13:33:18Z

Interesting...

the travis job for python 2.7 and null toolkit failed with the following traceback -

======================================================================

ERROR: test_text (kiva.tests.test_gl_drawing.TestGLDrawing)

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/tests/drawing_tester.py", line 72, in test_text

    self.gc.show_text("hello kiva")

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/gl.py", line 343, in show_text

    return self.show_text_at_point(text, *point)

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/gl.py", line 349, in show_text_at_point

    pyglet_font = GetFont(self._current_font)

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/gl.py", line 277, in GetFont

    pyglet_font = load_font(font.findfontname(), font.size, bold,

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/fonttools/font.py", line 115, in findfontname

    return fp.get_name()

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/fonttools/font_manager.py", line 878, in get_name

    return getPropDict(TTFont(str(font))['name'])

  File "/home/travis/.edm/envs/enable-test-2.7-null-pillow/lib/python2.7/site-packages/enable-4.8.1.dev19-py2.7-linux-x86_64.egg/kiva/fonttools/font_manager.py", line 246, in getPropDict

    n = font['name']

TypeError: 'table__n_a_m_e' object has no attribute '__getitem__'

-------------------- >> begin captured logging << --------------------

fontTools.ttLib.ttFont: DEBUG: Reading 'name' table from disk

fontTools.ttLib.ttFont: DEBUG: Decompiling 'name' table

--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------

Interesting... That is failing on one of the few lines in getPropDict that wasn't changed... Let me dig in some to see what I can find.

rahulporuri · 2019-09-24T14:08:11Z

Note that apart from the KeyError, I'm also seeing a UnicodeDecodeError like the following but this is probably a separate issue altogether.

2019-09-21 15:16:19,324 ERROR    MainThread [kiva.fonttools.font_manager:734] Cannot handle unicode file: /System/Library/Fonts/ヒラギノ角ゴシック W5.ttc

Traceback (most recent call last):

  File "/Users/travis/.edm/envs/XXX/lib/python2.7/site-packages/kiva/fonttools/font_manager.py", line 713, in createFontList

    collection = TTCollection(str(fpath))

UnicodeEncodeError: 'ascii' codec can't encode characters in position 22-32: ordinal not in range(128)

codecov-io · 2019-09-24T16:02:16Z

Codecov Report

Merging #365 into master will increase coverage by 0.46%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##           master     #365      +/-   ##
==========================================
+ Coverage   32.91%   33.37%   +0.46%     
==========================================
  Files         206      206              
  Lines       18303    18302       -1     
  Branches     2415     2416       +1     
==========================================
+ Hits         6024     6108      +84     
+ Misses      11893    11807      -86     
- Partials      386      387       +1

Impacted Files	Coverage Δ
kiva/fonttools/font_manager.py	`51.19% <77.77%> (+10.27%)`	⬆️
kiva/fonttools/font.py	`56.79% <0%> (+0.54%)`	⬆️
enable/tools/hover_tool.py	`37.31% <0%> (+2.98%)`	⬆️
kiva/trait_defs/kiva_font_trait.py	`76.56% <0%> (+3.12%)`	⬆️
enable/colors.py	`25% <0%> (+3.26%)`	⬆️
enable/component_editor.py	`79.16% <0%> (+4.16%)`	⬆️
enable/null/image.py	`85.71% <0%> (+85.71%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a843b37...893273a. Read the comment docs.

JCorson · 2019-09-24T16:43:32Z

I think I found the bug. I had the ['name'] on the TTFont rather than the return of getPropDict. Fixed in 841fbcb

JCorson · 2019-09-24T16:44:47Z

I also changed some str calls to six.text_type to ensure that filenames with unicode characters are handled on Python 2.7 (f73dfba)

rahulporuri · 2019-09-24T17:04:47Z


        font = fontManager.findfont(self)
-        return getPropDict(TTFont(six.text_type(font))['name'])
+        return getPropDict(TTFont(six.text_type(font)))['name']


do you think it will help readability if you change the formatting here to

return getPropDict( TTFont(six.text_type(font)) )['name']

or something of that sort?

Maybe even breaking that out of the return altogether to make it more readable. Something like:

prop_dict = getPropDict(TTFont(six.text_type(font)) return prop_dict['name']

What do you think @rahulporuri ?

mdickinson · 2019-09-24T18:52:31Z

Code changes LGTM; I'm running into bandwidth issues with local testing, though.

mdickinson · 2019-09-24T20:28:44Z

Local test results LGTM, too.

JCorson · 2019-09-25T13:18:07Z

Thanks all for the reviews!

FIX: KeyError while parsing ttfFontProperty

JCorson added 15 commits September 13, 2019 21:35

FIX: No bare exceptions

0b27e2e

FLK: Extra blank line

5b82bb9

FLK: Better variable name

a03e5ee

FLK: Extra blank line

2f3680b

TST: Add test font file from matplotlib

TST: Use test font from fonttools

f9597f6

TST: Add test for ttfFontProperty

1546790

FIX: Correct test method name

FIX: Simplify decoding of font props

2f45c8b

FIX: Add missing imports

FIX: Include ttf files in manifest

ffa4132

REF: Remove unneeded sfnt2 item

80f93bc

ENH: Break if both name and sfnt4 are found

f20e62e

ENH: Add function to handle utf-16 encoded font files

aa8d653

FIX: Remove __main__ block

REF: Decode prop when getting prop dictionary

1ebffac

FIX: Typo

2a17793

FIX: Better variable name

6150faf

FIX: Don't decode sfnt4 again

f754761

JCorson requested a review from mdickinson September 22, 2019 02:30

FIX: Include ttf file in egg

35444a4

FIX: Use six.texttype when converting to strings.

f73dfba

FIX: Get 'name' from prop dict, not TTFont.

841fbcb

rahulporuri reviewed Sep 24, 2019

View reviewed changes

FIX: Make getting name prop more readable

893273a

JCorson requested a review from jwiggins September 24, 2019 17:55

mdickinson approved these changes Sep 24, 2019

View reviewed changes

JCorson merged commit 5a9b2d1 into master Sep 25, 2019

JCorson deleted the fix/363-font-keyerror branch September 25, 2019 13:18

jwiggins pushed a commit that referenced this pull request Sep 30, 2019

Merge pull request #365 from enthought/fix/363-font-keyerror

d84511d

FIX: KeyError while parsing ttfFontProperty

jwiggins mentioned this pull request Sep 30, 2019

Release 4.8.1 #369

Merged

jwiggins mentioned this pull request Mar 10, 2021

Extract a better property dict from TTF fonts #697

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: KeyError while parsing ttfFontProperty#365

FIX: KeyError while parsing ttfFontProperty#365
JCorson merged 19 commits into
masterfrom
fix/363-font-keyerror

JCorson commented Sep 22, 2019 •

edited

Loading

Uh oh!

rahulporuri commented Sep 24, 2019

Uh oh!

JCorson commented Sep 24, 2019

Uh oh!

rahulporuri commented Sep 24, 2019 •

edited

Loading

Uh oh!

codecov-io commented Sep 24, 2019 •

edited

Loading

Uh oh!

JCorson commented Sep 24, 2019

Uh oh!

JCorson commented Sep 24, 2019

Uh oh!

rahulporuri Sep 24, 2019 •

edited

Loading

Uh oh!

JCorson Sep 24, 2019

Uh oh!

rahulporuri Sep 24, 2019

Uh oh!

mdickinson commented Sep 24, 2019

Uh oh!

mdickinson commented Sep 24, 2019

Uh oh!

JCorson commented Sep 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

JCorson commented Sep 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rahulporuri commented Sep 24, 2019

Uh oh!

JCorson commented Sep 24, 2019

Uh oh!

rahulporuri commented Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JCorson commented Sep 24, 2019

Uh oh!

JCorson commented Sep 24, 2019

Uh oh!

rahulporuri Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JCorson Sep 24, 2019

Choose a reason for hiding this comment

Uh oh!

rahulporuri Sep 24, 2019

Choose a reason for hiding this comment

Uh oh!

mdickinson commented Sep 24, 2019

Uh oh!

mdickinson commented Sep 24, 2019

Uh oh!

JCorson commented Sep 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JCorson commented Sep 22, 2019 •

edited

Loading

rahulporuri commented Sep 24, 2019 •

edited

Loading

codecov-io commented Sep 24, 2019 •

edited

Loading

rahulporuri Sep 24, 2019 •

edited

Loading