-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Use metric identifiers to parse an AFM character metric line #2783
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
Conversation
Travis build failed due to a problem in string_width_height, which I never touched, and because I was mixing binary and unicode strings. I've fixed the latter. |
@mdboom Can you take a look at this one? |
@jdeblese Can you take another look at this? It looks like |
Ok, I'll take another look. You're referring to the errors in the Travis build, right? It may just be the 'list()' bit that I dropped from line 209, but I'll need to set up my test environment again to confirm that. |
Ok, it was the missing "list()", and PEP8 compliance. Now passes the Travis checks. |
Are all of the explicit 'b' markers really needed? |
Other than that, this looks good to me 👍 . |
I had some issues in python 3 back in 2013 without them, if I recall
|
If it isn't too much trouble could you see which are not needed? My guess It is one of those things that once it is there, and people forget/don't Are there examples of afm's like this we can download as part of the On Sun, May 24, 2015 at 6:37 PM Jan-willem De Bleser <
|
The line I give as example comes from the STXihei font, which I converted to an afm using fontforge. Probably can't include that one as part of the test suite, but maybe we can find others under more open licenses. I'll take another look at where the 'b's might be needed. |
@jdeblese Awesome, thank you. |
Ok, so on my machine it complains if I remove any of the 'b' prefixes. The issue as I see it is that both the dictionary values as well as the keys are created directly from the byte literal string, without any conversion. Just using vals['WX'], for instance, will give me a key not found error, as 'WX' != b'WX'. Either I'll have to leave them all as byte literals, or I first have to convert 'line' to a unicode string. I can change the lambda inside the filter call to (len(s) > 0) instead of (s != b''), however. |
AFM files should be ASCII anyway, and the code already does a convert for 'name', so I pushed a commit that converts 'line' to a unicode string before any processing. Oddly enough Travis now fails for 2.6 with a file not found error on a part of the backend code, although not sure why. We can either dig out what happened there, or I can revert that last commit and we just live with the warts. Not much luck re: fonts. There's one or two Chinese fonts available under a GPL or Apache license, but I haven't tried seeing if they produce more complex metric files as I don't know if these could be distributed as part of the tests. |
@jdeblese Sorry this fell off my radar. The test failure was transient, we sometimes get funny things like that on travis (but it is free and generally awesome so it is ok). Restarted the test and it passed. Thanks for your work on this! |
FIX: Use metric identifiers to parse an AFM character metric line Don't make assumptions about order.
No problem, no rush on my part. Glad to hear it passes now, and if I can think of some test cases I'll make sure to pass them along. Thanks for merging it! |
An AFM Character Metric line contains key-value pairs, with some of these being mandatory and some optional. Matplotlib currently parses this using absolute positions, but there are no guarantees as to the order of keys on a line. For example, a Chinese font converted through FontForge results in an AFM with the following Character Metric:
C 0 ; WX 1000 ; WY 1377 ; N uni5200 ; B 73 -131 890 665 ;
The solution is to make use of the keys when parsing the metrics. Parsing a line out into a dictionary, as below, is one straightforward way.
Cheers,
Jw