Don't stop at first incompatibilty, log errors, raise at the end

Based on Miguel Sousa's original PR and the following discussion:
https://github.com/googlei18n/cu2qu/pull/114

Instead of raising an error at the first incompatible glyph, we
let it continue (keeping the original contours unmodified when
that happens), and use logging to print error messages.

A new `IncompatibleFontsError` exception is raised at the end of
`fonts_to_quadratic` if any glyph has incompatible number or types
of segments. The exception instance has a `glyph_errors` attribute
(dict) which collects all the individual IncompatibleGlyphsError
keyed by glyph name.

Lib/cu2qu/errors.py

@@ -0,0 +1,49 @@
+from __future__ import print_function, absolute_import, division
+
+
+class UnequalZipLengthsError(ValueError):
+    pass
+
+
+class IncompatibleGlyphsError(ValueError):
+
+    def __init__(self, glyphs):
+        assert len(glyphs) > 1
+        self.glyphs = glyphs
+        names = set(repr(g.name) for g in glyphs)
+        if len(names) > 1:
+            self.combined_name = "{%s}" % ", ".join(sorted(names))
+        else:
+            self.combined_name = names.pop()
+
+    def __repr__(self):
+        return "<%s %s>" % (type(self).__name__, self.combined_name)
+
+
+class IncompatibleSegmentNumberError(IncompatibleGlyphsError):
+
+    def __str__(self):
+        return "Glyphs named %s have different number of segments" % (
+            self.combined_name)
+
+
+class IncompatibleSegmentTypesError(IncompatibleGlyphsError):
+
+    def __init__(self, glyphs, segments):
+        IncompatibleGlyphsError.__init__(self, glyphs)
+        self.segments = segments
+
+    def __str__(self):
+        from pprint import pformat
+        return "Glyphs named %s have incompatible segment types:\n%s" % (
+            self.combined_name, pformat(self.segments))
+
+
+class IncompatibleFontsError(ValueError):
+
+    def __init__(self, glyph_errors):
+        self.glyph_errors = glyph_errors
+
+    def __str__(self):
+        return "fonts contains incompatible glyphs: %s" % (
+            ", ".join(repr(g) for g in sorted(self.glyph_errors.keys())))

Lib/cu2qu/ufo.py

@@ -31,6 +31,11 @@ from fontTools.pens.basePen import AbstractPen
 
 from cu2qu import curves_to_quadratic
 from cu2qu.pens import ReverseContourPen
+from cu2qu.errors import (
+    UnequalZipLengthsError, IncompatibleSegmentNumberError,
+    IncompatibleSegmentTypesError, IncompatibleGlyphsError,
+    IncompatibleFontsError)
+
 
 __all__ = ['fonts_to_quadratic', 'font_to_quadratic']
 
@@ -39,16 +44,6 @@ DEFAULT_MAX_ERR = 0.001
 logger = logging.getLogger(__name__)
 
 
-class IncompatibleGlyphsError(ValueError):
-
-    def __str__(self):
-        return ", ".join(set(repr(glyph.name) for glyph in self.args))
-
-
-class UnequalZipLengthsError(ValueError):
-    pass
-
-
 _zip = zip
 def zip(*args):
     """Ensure each argument to zip has the same length. Also make sure a list is
@@ -153,7 +148,7 @@ def _glyphs_to_quadratic(glyphs, max_err, reverse_direction, stats):
     try:
         segments_by_location = zip(*[_get_segments(g) for g in glyphs])
     except UnequalZipLengthsError:
-        raise IncompatibleGlyphsError(*glyphs)
+        raise IncompatibleSegmentNumberError(glyphs)
     if not any(segments_by_location):
         return False
 
@@ -161,11 +156,12 @@ def _glyphs_to_quadratic(glyphs, max_err, reverse_direction, stats):
     glyphs_modified = reverse_direction
 
     new_segments_by_location = []
-    for segments in segments_by_location:
+    incompatible = {}
+    for i, segments in enumerate(segments_by_location):
         tag = segments[0][0]
         if not all(s[0] == tag for s in segments[1:]):
-            raise IncompatibleGlyphsError(*glyphs)
+            incompatible[i] = [s[0] for s in segments]
-        if tag == 'curve':
+        elif tag == 'curve':
             segments = _segments_to_quadratic(segments, max_err, stats)
             glyphs_modified = True
         new_segments_by_location.append(segments)
@@ -175,6 +171,8 @@ def _glyphs_to_quadratic(glyphs, max_err, reverse_direction, stats):
         for glyph, new_segments in zip(glyphs, new_segments_by_glyph):
             _set_segments(glyph, new_segments, reverse_direction)
 
+    if incompatible:
+        raise IncompatibleSegmentTypesError(glyphs, segments=incompatible)
     return glyphs_modified
 
 
@@ -217,7 +215,7 @@ def fonts_to_quadratic(
 
     Return True if fonts were modified, else return False.
 
-    Raises IncompatibleGlyphsError if same-named glyphs from different fonts
+    Raises IncompatibleFontsError if same-named glyphs from different fonts
     have non-interpolatable outlines.
     """
 
@@ -243,6 +241,7 @@ def fonts_to_quadratic(
         max_errors = [f.info.unitsPerEm * max_err_em for f in fonts]
 
     modified = False
+    glyph_errors = {}
     for name in set().union(*(f.keys() for f in fonts)):
         glyphs = []
         cur_max_errors = []
@@ -250,8 +249,15 @@ def fonts_to_quadratic(
             if name in font:
                 glyphs.append(font[name])
                 cur_max_errors.append(error)
-        modified |= _glyphs_to_quadratic(
+        try:
-            glyphs, cur_max_errors, reverse_direction, stats)
+            modified |= _glyphs_to_quadratic(
+                glyphs, cur_max_errors, reverse_direction, stats)
+        except IncompatibleGlyphsError as exc:
+            logger.error(exc)
+            glyph_errors[name] = exc
+
+    if glyph_errors:
+        raise IncompatibleFontsError(glyph_errors)
 
     if modified and dump_stats:
         spline_lengths = sorted(stats.keys())

tests/ufo_test.py

@@ -9,7 +9,11 @@ from cu2qu.ufo import (
     glyphs_to_quadratic,
     glyph_to_quadratic,
     logger,
-    IncompatibleGlyphsError
+)
+from cu2qu.errors import (
+    IncompatibleSegmentNumberError,
+    IncompatibleSegmentTypesError,
+    IncompatibleFontsError,
 )
 
 from . import DATADIR
@@ -116,40 +120,49 @@ class GlyphsToQuadraticTest(object):
                                   reverse_direction=True)
 
     @pytest.mark.parametrize(
-        "outlines",
+        ["outlines", "exception", "message"],
         [
             [
                 [
-                    ('moveTo', ((0, 0),)),
+                    [
-                    ('curveTo', ((1, 1), (2, 2), (3, 3))),
+                        ('moveTo', ((0, 0),)),
-                    ('curveTo', ((4, 4), (5, 5), (6, 6))),
+                        ('curveTo', ((1, 1), (2, 2), (3, 3))),
-                    ('closePath', ()),
+                        ('curveTo', ((4, 4), (5, 5), (6, 6))),
+                        ('closePath', ()),
+                    ],
+                    [
+                        ('moveTo', ((7, 7),)),
+                        ('curveTo', ((8, 8), (9, 9), (10, 10))),
+                        ('closePath', ()),
+                    ]
                 ],
-                [
+                IncompatibleSegmentNumberError,
-                    ('moveTo', ((7, 7),)),
+                "have different number of segments",
-                    ('curveTo', ((8, 8), (9, 9), (10, 10))),
-                    ('closePath', ()),
-                ]
             ],
             [
                 [
-                    ('moveTo', ((0, 0),)),
+
-                    ('curveTo', ((1, 1), (2, 2), (3, 3))),
+                    [
-                    ('closePath', ()),
+                        ('moveTo', ((0, 0),)),
+                        ('curveTo', ((1, 1), (2, 2), (3, 3))),
+                        ('closePath', ()),
+                    ],
+                    [
+                        ('moveTo', ((4, 4),)),
+                        ('lineTo', ((5, 5),)),
+                        ('closePath', ()),
+                    ],
                 ],
-                [
+                IncompatibleSegmentTypesError,
-                    ('moveTo', ((4, 4),)),
+                "have incompatible segment types",
-                    ('lineTo', ((5, 5),)),
+            ],
-                    ('closePath', ()),
-                ],
-            ]
         ],
         ids=[
             "unequal-length",
             "different-segment-types",
         ]
     )
-    def test_incompatible(self, outlines):
+    def test_incompatible_glyphs(self, outlines, exception, message):
         glyphs = []
         for i, outline in enumerate(outlines):
             glyph = Glyph()
@@ -158,9 +171,37 @@ class GlyphsToQuadraticTest(object):
             for operator, args in outline:
                 getattr(pen, operator)(*args)
             glyphs.append(glyph)
-        with pytest.raises(IncompatibleGlyphsError) as excinfo:
+        with pytest.raises(exception) as excinfo:
             glyphs_to_quadratic(glyphs)
-        assert excinfo.match("^'glyph[0-9]+'(, 'glyph[0-9]+')*$")
+        assert excinfo.match(message)
+
+    def test_incompatible_fonts(self):
+        font1 = Font()
+        font1.info.unitsPerEm = 1000
+        glyph1 = font1.newGlyph("a")
+        pen1 = glyph1.getPen()
+        for operator, args in [("moveTo", ((0, 0),)),
+                               ("lineTo", ((1, 1),)),
+                               ("endPath", ())]:
+            getattr(pen1, operator)(*args)
+
+        font2 = Font()
+        font2.info.unitsPerEm = 1000
+        glyph2 = font2.newGlyph("a")
+        pen2 = glyph2.getPen()
+        for operator, args in [("moveTo", ((0, 0),)),
+                               ("curveTo", ((1, 1), (2, 2), (3, 3))),
+                               ("endPath", ())]:
+            getattr(pen2, operator)(*args)
+
+        with pytest.raises(IncompatibleFontsError) as excinfo:
+            fonts_to_quadratic([font1, font2])
+        assert excinfo.match("fonts contains incompatible glyphs: 'a'")
+
+        assert hasattr(excinfo.value, "glyph_errors")
+        error = excinfo.value.glyph_errors['a']
+        assert isinstance(error, IncompatibleSegmentTypesError)
+        assert error.segments == {1: ["line", "curve"]}
 
     def test_already_quadratic(self):
         glyph = Glyph()