Repository: googlefonts/glyphsLib

Issue:
glyph_data has contradictory types
`glyphsLib.builder.to_ufos` takes a `glyph_data` parameter which is (a) passed to `preflight_glyphs`, and (b) passed to `UFOBuilder`.

In `preflight_glyphs`, `glyph_data` is interpreted as:

https://github.com/googlefonts/glyphsLib/blob/8698f2c9411aaa0ff992a34988cc684b8dac62b0/Lib/glyphsLib/builder/__init__.py#L170-L171

So it's a `GlyphData` object. However, in `UFOBuilder`, it is described as

https://github.com/googlefonts/glyphsLib/blob/8698f2c9411aaa0ff992a34988cc684b8dac62b0/Lib/glyphsLib/builder/builders.py#L88

What does that mean?

https://github.com/googlefonts/glyphsLib/blob/8698f2c9411aaa0ff992a34988cc684b8dac62b0/Lib/glyphsLib/builder/builders.py#L187-L194

Oh, it's a list of *paths*.

It's historically been a list of paths, but now if you pass a list of paths, `preflight_glyphs` crashes because it's expecting a `GlyphData` object:

```
  File "/Users/simon/work/noto/math/venv/lib/python3.12/site-packages/glyphsLib/builder/transformations/propagate_anchors.py", line 67, in propagate_all_anchors_impl
    anchors = anchors_traversing_components(
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/simon/work/noto/math/venv/lib/python3.12/site-packages/glyphsLib/builder/transformations/propagate_anchors.py", line 157, in anchors_traversing_components
    if layer.anchors and _get_category(glyph, glyph_data) == "Mark":
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/simon/work/noto/math/venv/lib/python3.12/site-packages/glyphsLib/builder/transformations/propagate_anchors.py", line 113, in _get_category
    or glyphdata.get_glyph(
       ^^^^^^^^^^^^^^^^^^^^
  File "/Users/simon/work/noto/math/venv/lib/python3.12/site-packages/glyphsLib/glyphdata.py", line 121, in get_glyph
    attributes = _lookup_attributes(glyph_name, data)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/simon/work/noto/math/venv/lib/python3.12/site-packages/glyphsLib/glyphdata.py", line 164, in _lookup_attributes
    data.names.get(glyph_name)
    ^^^^^^^^^^
AttributeError: 'list' object has no attribute 'names'
```



Discussion:

Patch:
diff --git a/Lib/glyphsLib/builder/__init__.py b/Lib/glyphsLib/builder/__init__.py
index 1dcfe027..263d4d86 100644
--- a/Lib/glyphsLib/builder/__init__.py
+++ b/Lib/glyphsLib/builder/__init__.py
@@ -15,7 +15,7 @@
 import copy
 import logging
 
-from glyphsLib import classes
+from glyphsLib import classes, glyphdata
 
 from .builders import UFOBuilder, GlyphsBuilder
 from .transformations import TRANSFORMATIONS, TRANSFORMATION_CUSTOM_PARAMS
@@ -61,9 +61,14 @@ def to_ufos(
 
     If preserve_original is True, this works on a copy of the font object
     to avoid modifying the original object.
+
+    The optional glyph_data parameter takes a list of GlyphData.xml paths or
+    a pre-parsed GlyphData object that overrides the default one.
     """
     if preserve_original:
         font = copy.deepcopy(font)
+    if glyph_data is not None and not isinstance(glyph_data, glyphdata.GlyphData):
+        glyph_data = glyphdata.GlyphData.from_files(*glyph_data)
     font = preflight_glyphs(
         font, glyph_data=glyph_data, do_propagate_all_anchors=propagate_anchors
     )
@@ -128,9 +133,14 @@ def to_designspace(
 
     If preserve_original is True, this works on a copy of the font object
     to avoid modifying the original object.
+
+    The optional glyph_data parameter takes a list of GlyphData.xml paths or
+    a pre-parsed GlyphData object that overrides the default one.
     """
     if preserve_original:
         font = copy.deepcopy(font)
+    if glyph_data is not None and not isinstance(glyph_data, glyphdata.GlyphData):
+        glyph_data = glyphdata.GlyphData.from_files(*glyph_data)
     font = preflight_glyphs(
         font, glyph_data=glyph_data, do_propagate_all_anchors=propagate_anchors
     )
diff --git a/Lib/glyphsLib/builder/builders.py b/Lib/glyphsLib/builder/builders.py
index 70c1d401..460a1750 100644
--- a/Lib/glyphsLib/builder/builders.py
+++ b/Lib/glyphsLib/builder/builders.py
@@ -184,16 +184,9 @@ class UFOBuilder(LoggerMixin):
             # instances with matching 'familyName' custom parameter
             self._do_filter_instances_by_family = True
 
-        if glyph_data:
-            from io import BytesIO
-
-            glyphdata_files = []
-            for path in glyph_data:
-                with open(path, "rb") as fp:
-                    glyphdata_files.append(BytesIO(fp.read()))
-            self.glyphdata = glyphdata.GlyphData.from_files(*glyphdata_files)
-        else:
-            self.glyphdata = None
+        if glyph_data is not None and not isinstance(glyph_data, glyphdata.GlyphData):
+            glyph_data = glyphdata.GlyphData.from_files(*glyph_data)
+        self.glyphdata = glyph_data
 
     def _is_vertical(self):
         master_ids = {m.id for m in self.font.masters}


Test patch:
diff --git a/tests/builder/builder_test.py b/tests/builder/builder_test.py
index aaa39286..c32f870f 100644
--- a/tests/builder/builder_test.py
+++ b/tests/builder/builder_test.py
@@ -2298,6 +2298,8 @@ def test_custom_glyph_data(ufo_module):
     font = generate_minimal_font()
     for glyph_name in ("A", "Aitalic-math", "Aitalic-math.ssty1", "foo", "bar", "baz"):
         add_glyph(font, glyph_name)
+    # add a composite glyph to trigger propagate_anchors
+    add_component(font, "bar", "baz", (1, 0, 0, 1, 0, 0))
     font.glyphs["baz"].production = "bazglyph"
     font.glyphs["baz"].category = "Number"
     font.glyphs["baz"].subCategory = "Decimal Digit"