Repository: googlefonts/glyphsLib
Pull request #630 (2020-11-04 11:07:10)

Fix problem where axis with no mappings was erroring out during roundtrip
Problem: during `to_designspace` execution in the `to_glyphs`, `to_designspace` roundtrip, an error occurs when a custom axis contains no mappings.

`ValueError: min() arg is an empty sequence`

This occurs because Glyphs.app does not store an axis' `minimum`, `maximum` and `default` values, and therefore these values are wiped during `to_glyphs` conversion. Normally, `to_designspace` recalculates these values using the sources and the axis' mapping values. However if there are no mappings in the axis, `to_designspace` will error.

This error can be avoided by adding identity mappings to the designspace, as `to_glyphs` will keep these mappings, which then provides `to_designspace` with the required mappings it needs to calculate the minimum and maximum values of the custom axis. However, during runtime `to_designspace` deletes all identity mappings, thus causing the roundtrip to fail.

The fix is to make `to_glyphs` create an identity mapping if one is needed (when the are no axis mappings provided in the designspace) using the axis' minimum and maximum values, so that this information is not lost. This identity mapping can then be used to recalculate the min, max values on the next `to_designspace` trip (which still deletes the identity mappings). Thus allowing roundtrips to not lose information.

Summary:
* `to_glyphs` now stores axis min, max information as identity mappings to stop the loss of this information which previously caused errors during roundtrip.


Patch:
diff --git a/Lib/glyphsLib/builder/builders.py b/Lib/glyphsLib/builder/builders.py
index 8d4b440e..78b626bd 100644
--- a/Lib/glyphsLib/builder/builders.py
+++ b/Lib/glyphsLib/builder/builders.py
@@ -749,6 +749,19 @@ class GlyphsBuilder(_LoggerMixin):
                         )
                     )
                     setattr(source, name, getattr(source.font.info, name))
+        # An axis without a mapping will see its range information (min and max
+        # values) lost when converted to a Glyps.app file. To combat this we
+        # add an explicit identity mapping.
+        for axis in copy.axes:
+            if axis.map:
+                continue
+            if axis.minimum == axis.maximum:
+                axis.map = [(axis.minimum, axis.minimum)]
+            else:
+                axis.map = [
+                    (axis.minimum, axis.minimum),
+                    (axis.maximum, axis.maximum),
+                ]
         return copy
 
     def _fake_designspace(self, ufos):


Test patch:
diff --git a/tests/builder/axes_test.py b/tests/builder/axes_test.py
index 06423ac4..a3f49ccf 100644
--- a/tests/builder/axes_test.py
+++ b/tests/builder/axes_test.py
@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from copy import deepcopy
 import os.path
 
 import pytest
@@ -81,6 +82,9 @@ def _make_designspace_with_axes(axes, ufo_module):
         axis = doc.newAxisDescriptor()
         axis.tag = tag
         axis.name = name
+        axis.minimum = 0
+        axis.default = 0
+        axis.maximum = 100
         doc.addAxis(axis)
 
         extreme = doc.newSourceDescriptor()
@@ -347,8 +351,60 @@ def test_axis_mapping(ufo_module):
     assert doc.axes[1].map != wdth_mapping
     assert doc.axes[1].map == []
 
-    # When we convert back to glyphs, the wdth mapping isn't present because
-    # it was removed during the designspace conversion.
     font = to_glyphs(doc)
-    axis_mappings["wdth"] = {}
     assert font.customParameters["Axis Mappings"] == axis_mappings
+
+
+def test_axis_with_no_mapping_does_not_error_in_roundtrip(ufo_module):
+    """Tests that a custom axis without a mapping and without sources on its
+    extremes does not generate an error during roundtrip. Also tests that
+    during a to_glyphs, to_designspace roundtrip the min and max axis
+    information is not lost.
+    """
+    doc = designspaceLib.DesignSpaceDocument()
+
+    # Add a "Regular" source
+    regular = doc.newSourceDescriptor()
+    regular.font = ufo_module.Font()
+    regular.location = {"Style": 0}
+    doc.addSource(regular)
+
+    axis = doc.newAxisDescriptor()
+    axis.tag = "styl"
+    axis.name = "Style"
+    doc.addAxis(axis)
+
+    # This axis spans a range of 0 to 1 but only has a source at {"Style": 0}
+    # and no explicit mapping. The point of this test is to see if the min and
+    # max are still the same after round tripping.
+    doc.axes[0].minimum = 0
+    doc.axes[0].maximum = 1
+    doc.axes[0].default = 0
+    doc.axes[0].map = []
+
+    doc2 = deepcopy(doc)
+    font = to_glyphs(doc2)
+    doc_rt = to_designspace(font)
+
+    assert doc_rt.axes[0].serialize() == doc.axes[0].serialize()
+
+
+def test_axis_with_no_mapping_does_not_error_in_roundtrip_with_2_axes(ufo_module):
+    """Tests that a designspace with 2 axis, one with a mapping and one
+    without a mapping, roundtrips correctly without error. The axis without a
+    mapping should generate an identity mapping on the fly so that the
+    Glyphs.app customParameter field does not lose min/max infromation about
+    the axis.
+    """
+    doc = _make_designspace_with_axes(
+        [("wght", "Weight with mapping"), ("wdth", "Width without mapping")], ufo_module
+    )
+    # Add mapping to weight axis
+    doc.axes[0].map = [(0, 0), (50, 350), (100, 1000)]
+
+    doc2 = deepcopy(doc)
+    font = to_glyphs(doc2)
+    doc_rt = to_designspace(font)
+
+    assert doc_rt.axes[0].serialize() == doc.axes[0].serialize()
+    assert doc_rt.axes[1].serialize() == doc.axes[1].serialize()