Kerning is unstructured as Dict[rapidjson.RawJSON, int], making JSON dumping fail about tfont HOT 14 CLOSED

Loading the file still fails though:

Traceback (most recent call last):
  File "/home/nikolaus/Entwicklung/trufont/src/trufont/windows/fontWindow.py", line 584, in OnOpen
    trufont.TruFont.openFont()
  File "/home/nikolaus/Entwicklung/trufont/src/trufont/objects/application.py", line 127, in openFont
    font = TFontConverter().open(path)
  File "/home/nikolaus/Entwicklung/tfont/src/tfont/converters/tfontConverter.py", line 138, in open
    return self.structure(d, Font)
  File "/home/nikolaus/.virtualenvs/trufont/lib64/python3.7/site-packages/cattr/converters.py", line 178, in structure
    return self._structure_func.dispatch(cl)(obj, cl)
  File "/home/nikolaus/Entwicklung/tfont/src/tfont/converters/tfontConverter.py", line 161, in structure_attrs_fromdict
    conv_obj[name] = dispatch(type_)(val, type_)
  File "/home/nikolaus/Entwicklung/tfont/src/tfont/converters/tfontConverter.py", line 22, in _structure_seq_dict
    return dict((e[attr], self.structure(e, cls)) for e in data)
  File "/home/nikolaus/Entwicklung/tfont/src/tfont/converters/tfontConverter.py", line 22, in <genexpr>
    return dict((e[attr], self.structure(e, cls)) for e in data)
TypeError: string indices must be integers

data is:

{'Bold': {'alignmentZones': [[-14, 14], [526, 14], [693, 16], [-188, 3]],
          'ascender': 776,
          'capHeight': 693,
          'descender': -185,
          'guidelines': [{'angle': 0, 'x': 0, 'y': 346},
                         {'angle': 0, 'x': 0, 'y': 736},
                         {'angle': 0, 'x': 0, 'y': 962}],
          'name': 'Bold',
          'xHeight': 526}}

In this case, attr is "name", so I suppose data should have been the dict of 'Bold'.

Here's the test tfont file (no kerning): test.zip

from tfont.

This is what broke it: fonttools/ufoLib2@50dec8c (kerning changed format)

Should use my original ufoLib2 w/o the breaking changes to make it behave like previous ufo libraries. e.g. the font I showed you before was made with the original and has kerning that works (which is why I found this report surprising).

from tfont.

Oh boy. So, which ufoLib2 should be the canonical one?

@anthrotype

from tfont.

I changed ufoLib2 readKerning in that commit so that it matched the behaviour of ufoLib, which returns a single dictionary keyed by (left, right) tuples, instead of nested dictionaries.
If tfont needs it the other way, it should be trivial to recreate that nested dict structure upon doing the conversion from UFO.
Note that once we move ufoLib to fontTools, and add the objects API similar to defcon, then ufoLib2 has no more reason to exist as standalone module and we shall move tfont to use fontTools.ufoLib instead.

from tfont.

from tfont.

this should work. Shall I send a PR?

diff --git a/src/tfont/converters/ufoConverter.py b/src/tfont/converters/ufoConverter.py
index da18127..53a7b71 100644
--- a/src/tfont/converters/ufoConverter.py
+++ b/src/tfont/converters/ufoConverter.py
@@ -74,7 +74,12 @@ class UFOConverter(cattr.Converter):
         # note: unlike ufo, we store kerning in visual order. hard to convert
         # between the two (given that ltr and rtl pairs can be mixed)
         if ufo.kerning:
-            master.hKerning = ufo.kerning
+            hKerning = {}
+            for (first, second), value in ufo.kerning.items():
+                if first not in hKerning:
+                    hKerning[first] = {}
+                hKerning[first][second] = value
+            master.hKerning = hKerning
         if info.ascender:
             master.ascender = info.ascender
         if info.capHeight:

from tfont.

I just noticed ufoLib2 is imported at the module-level scope inside tfont.converters.ufoConverter but it is not specified in the setup.py install_requires:

from tfont.

apart from the hKerning issue, the other one that @madig pointed out is unrelated to ufoLib2 and I can repro with this simple code:

from tfont.converters import TFontConverter
from tfont.objects import Font

converter = TFontConverter()
font = Font()
converter.save(font, "test.tfont")
font2 = converter.open("test.tfont")

from tfont.

I just noticed ufoLib2 ... is not specified in the setup.py install_requires:

I just added that to setup.py as part of #4
d206a6d

from tfont.

If you prefer to keep ufoLib2 optional instead of required we can add it to extras_require under a key named “ufo”. So pip install tfont[ufo] will also install ufoLib2 and its deps.

But then we need to avoid importing ufoConverter module until it’s needed, otherwise we get an ImportError

from tfont.

sgtm

he other one that @madig pointed out

this is due to python/typing#570 apparently (pep560/python 3.7 breaking change)

from tfont.

cebbd38

from tfont.

this is due to python/typing#570 apparently (pep560/python 3.7 breaking change)

@adrientetar ah! Now I remember.. Thanks for the quick fix.

sgtm

Ok, I'll move ufoLib2 to extras_require. Shall I also apply the un-flattening of hKerning dict?

from tfont.

yeah

from tfont.