Repository: googlefonts/glyphsLib
Pull request #644 (2020-12-02 13:11:58)

Improve compatibility with Glyphs
When trying to run a Glyphs script on the command line, I noticed that I couldn't set a node's position. Glyphs allows `n.position = (x,y)`, but this creates a tuple when GSNode expects a `Point` object. But of course I can't use a `Point` object because then the script wouldn't work in Glyphs...

This PR allows setting `n.position = (x,y)` and automatically upcasting that to a `Point` object.


Patch:
diff --git a/Lib/glyphsLib/builder/builders.py b/Lib/glyphsLib/builder/builders.py
index 939c2d4c..8d4b440e 100644
--- a/Lib/glyphsLib/builder/builders.py
+++ b/Lib/glyphsLib/builder/builders.py
@@ -130,13 +130,6 @@ class UFOBuilder(_LoggerMixin):
         self._designspace = self.designspace_module.DesignSpaceDocument()
         self._designspace_is_complete = False
 
-        # If any glyph layer which is associated with a GSFontMaster has the
-        # properties "vertWidth" or "vertOrigin" set, Glyphsapp will assume
-        # the font is going to be used for vertical typesetting. When
-        # Glyphsapp generates these fonts, it will include the vhea, vmtx,
-        # VORG tables. VORG will only be included if the font is an otf.
-        self.is_vertical = self._is_vertical()
-
         # check that source was generated with at least stable version 2.3
         # https://github.com/googlefonts/glyphsLib/pull/65#issuecomment-237158140
         if int(font.appVersion) < 895:
@@ -167,16 +160,6 @@ class UFOBuilder(_LoggerMixin):
             # instances with matching 'familyName' custom parameter
             self._do_filter_instances_by_family = True
 
-    def _is_vertical(self):
-        master_ids = {m.id for m in self.font.masters}
-        for glyph in self.font.glyphs:
-            for layer in glyph.layers:
-                if layer.layerId not in master_ids:
-                    continue
-                if layer.vertWidth is not None or layer.vertOrigin is not None:
-                    return True
-        return False
-
     @property
     def masters(self):
         """Get an iterator over master UFOs that match the given family_name.
@@ -517,6 +500,7 @@ class UFOBuilder(_LoggerMixin):
     from .custom_params import to_ufo_custom_params
     from .features import to_ufo_features
     from .font import to_ufo_font_attributes
+    from .glyph import to_ufo_glyph, to_ufo_glyph_background
     from .groups import to_ufo_groups
     from .guidelines import to_ufo_guidelines
     from .hints import to_ufo_hints
@@ -527,11 +511,6 @@ class UFOBuilder(_LoggerMixin):
     from .names import to_ufo_names
     from .paths import to_ufo_paths
     from .sources import to_designspace_sources
-    from .glyph import (
-        to_ufo_glyph,
-        to_ufo_glyph_background,
-        to_ufo_glyph_height_and_vertical_origin,
-    )
     from .user_data import (
         to_designspace_family_user_data,
         to_ufo_family_user_data,
@@ -770,19 +749,6 @@ 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):
@@ -859,7 +825,7 @@ class GlyphsBuilder(_LoggerMixin):
     from .custom_params import to_glyphs_custom_params
     from .features import to_glyphs_features
     from .font import to_glyphs_font_attributes, to_glyphs_ordered_masters
-    from .glyph import to_glyphs_glyph, to_glyphs_glyph_height_and_vertical_origin
+    from .glyph import to_glyphs_glyph
     from .groups import to_glyphs_groups
     from .guidelines import to_glyphs_guidelines
     from .hints import to_glyphs_hints
diff --git a/Lib/glyphsLib/builder/custom_params.py b/Lib/glyphsLib/builder/custom_params.py
index b82f8ff5..78578924 100644
--- a/Lib/glyphsLib/builder/custom_params.py
+++ b/Lib/glyphsLib/builder/custom_params.py
@@ -322,9 +322,6 @@ GLYPHS_UFO_CUSTOM_PARAMS = (
     # ('widthClass', 'openTypeOS2WidthClass'),
     # ('winAscent', 'openTypeOS2WinAscent'),
     # ('winDescent', 'openTypeOS2WinDescent'),
-    ("vheaVertAscender", "openTypeVheaVertTypoAscender"),
-    ("vheaVertDescender", "openTypeVheaVertTypoDescender"),
-    ("vheaVertLineGap", "openTypeVheaVertTypoLineGap"),
     ("vheaVertTypoAscender", "openTypeVheaVertTypoAscender"),
     ("vheaVertTypoDescender", "openTypeVheaVertTypoDescender"),
     ("vheaVertTypoLineGap", "openTypeVheaVertTypoLineGap"),
diff --git a/Lib/glyphsLib/builder/glyph.py b/Lib/glyphsLib/builder/glyph.py
index f3a355e0..cb9f1744 100644
--- a/Lib/glyphsLib/builder/glyph.py
+++ b/Lib/glyphsLib/builder/glyph.py
@@ -26,7 +26,7 @@ ORIGINAL_WIDTH_KEY = GLYPHLIB_PREFIX + "originalWidth"
 BACKGROUND_WIDTH_KEY = GLYPHLIB_PREFIX + "backgroundWidth"
 
 
-def to_ufo_glyph(self, ufo_glyph, layer, glyph):  # noqa: C901
+def to_ufo_glyph(self, ufo_glyph, layer, glyph):
     """Add .glyphs metadata, paths, components, and anchors to a glyph."""
     ufo_font = self._sources[layer.associatedMasterId or layer.layerId].font
 
@@ -138,8 +138,6 @@ def to_ufo_glyph(self, ufo_glyph, layer, glyph):  # noqa: C901
     self.to_ufo_paths(ufo_glyph, layer)
     self.to_ufo_components(ufo_glyph, layer)
     self.to_ufo_glyph_anchors(ufo_glyph, layer.anchors)
-    if self.is_vertical:
-        self.to_ufo_glyph_height_and_vertical_origin(ufo_glyph, layer)
 
 
 def to_glyphs_glyph(self, ufo_glyph, ufo_layer, master):  # noqa: C901
@@ -258,40 +256,6 @@ def to_glyphs_glyph(self, ufo_glyph, ufo_layer, master):  # noqa: C901
     self.to_glyphs_paths(ufo_glyph, layer)
     self.to_glyphs_components(ufo_glyph, layer)
     self.to_glyphs_glyph_anchors(ufo_glyph, layer)
-    self.to_glyphs_glyph_height_and_vertical_origin(ufo_glyph, master, layer)
-
-
-def to_ufo_glyph_height_and_vertical_origin(self, ufo_glyph, layer):
-    # implentation based on:
-    # https://github.com/googlefonts/glyphsLib/issues/557#issuecomment-667074856
-    assert self.is_vertical
-
-    ascender, descender = _get_typo_ascender_descender(layer.master)
-
-    if layer.vertWidth:
-        ufo_glyph.height = layer.vertWidth
-    else:
-        ufo_glyph.height = ascender - descender
-
-    if layer.vertOrigin:
-        ufo_glyph.verticalOrigin = ascender - layer.vertOrigin
-    else:
-        ufo_glyph.verticalOrigin = ascender
-
-
-def _get_typo_ascender_descender(master):
-    # Glyphsapp will use the typo metrics to set the verOrigin and
-    # vertWidth. If typo metrics are not present, the master
-    # ascender and descender are used instead.
-    if "typoAscender" in master.customParameters:
-        ascender = master.customParameters["typoAscender"]
-    else:
-        ascender = master.ascender
-    if "typoDescender" in master.customParameters:
-        descender = master.customParameters["typoDescender"]
-    else:
-        descender = master.descender
-    return ascender, descender
 
 
 def to_ufo_glyph_background(self, glyph, layer):
@@ -327,12 +291,3 @@ def _to_glyphs_color(color):
     # round-trip the actual value in later versions.
     # https://github.com/googlefonts/glyphsLib/pull/363#issuecomment-390418497
     return [round(float(component) * 255) for component in color.split(",")]
-
-
-def to_glyphs_glyph_height_and_vertical_origin(self, ufo_glyph, master, layer):
-    if ufo_glyph.height:
-        layer.vertWidth = ufo_glyph.height
-
-    if ufo_glyph.verticalOrigin:
-        ascender, _ = _get_typo_ascender_descender(master)
-        layer.vertOrigin = ascender - ufo_glyph.verticalOrigin
diff --git a/Lib/glyphsLib/builder/masters.py b/Lib/glyphsLib/builder/masters.py
index 6c3e4f3f..2a8682e4 100644
--- a/Lib/glyphsLib/builder/masters.py
+++ b/Lib/glyphsLib/builder/masters.py
@@ -71,26 +71,6 @@ def to_ufo_master_attributes(self, source, master):
                 user_loc = axis.get_user_loc(master)
                 axis.set_ufo_user_loc(ufo, user_loc)
 
-    # Set vhea values to glyphsapp defaults if they haven't been declared.
-    # ufo2ft needs these set in order for a ufo to be recognised as
-    # vertical. Glyphsapp uses the font upm, not the typo metrics
-    # for these.
-    if self.is_vertical:
-        custom_params = master.customParameters
-        font_upm = self.font.upm
-        if not any(
-            k in custom_params for k in ("vheaVertAscender", "vheaVertTypoAscender")
-        ):
-            ufo.info.openTypeVheaVertTypoAscender = int(font_upm / 2)
-        if not any(
-            k in custom_params for k in ("vheaVertDescender", "vheaVertTypoDescender")
-        ):
-            ufo.info.openTypeVheaVertTypoDescender = -int(font_upm / 2)
-        if not any(
-            k in custom_params for k in ("vheaVertLineGap", "vheaVertTypoLineGap")
-        ):
-            ufo.info.openTypeVheaVertTypoLineGap = font_upm
-
     self.to_ufo_blue_values(ufo, master)
     self.to_ufo_guidelines(ufo, master)
     self.to_ufo_master_user_data(ufo, master)
diff --git a/Lib/glyphsLib/classes.py b/Lib/glyphsLib/classes.py
index b1595979..17ed4609 100755
--- a/Lib/glyphsLib/classes.py
+++ b/Lib/glyphsLib/classes.py
@@ -686,8 +686,6 @@ class FontGlyphsProxy(Proxy):
 
 
 class FontClassesProxy(Proxy):
-    VALUES_ATTR = "_classes"
-
     def __getitem__(self, key):
         if isinstance(key, (slice, int)):
             return self.values().__getitem__(key)
@@ -717,13 +715,7 @@ class FontClassesProxy(Proxy):
                 if klass.name == key:
                     del self.values()[index]
 
-    def __contains__(self, item):
-        if isinstance(item, str):
-            for klass in self.values():
-                if klass.name == item:
-                    return True
-            return False
-        return item in self.values()
+    # FIXME: (jany) def __contains__
 
     def append(self, item):
         self.values().append(item)
@@ -742,24 +734,16 @@ class FontClassesProxy(Proxy):
         self.values().remove(item)
 
     def values(self):
-        return getattr(self._owner, self.VALUES_ATTR)
+        return self._owner._classes
 
     def setter(self, values):
         if isinstance(values, Proxy):
             values = list(values)
-        setattr(self._owner, self.VALUES_ATTR, values)
+        self._owner._classes = values
         for value in values:
             value._parent = self._owner
 
 
-class FontFeaturesProxy(FontClassesProxy):
-    VALUES_ATTR = "_features"
-
-
-class FontFeaturePrefixesProxy(FontClassesProxy):
-    VALUES_ATTR = "_featurePrefixes"
-
-
 class GlyphLayerProxy(Proxy):
     def __getitem__(self, key):
         self._ensureMasterLayers()
@@ -1619,7 +1603,7 @@ class GSFontMaster(GSBase):
 
 
 class GSNode(GSBase):
-    __slots__ = ("_userData", "position", "smooth", "type")
+    __slots__ = ("_userData", "_position", "smooth", "type")
 
     _PLIST_VALUE_RE = re.compile(
         r'"([-.e\d]+) ([-.e\d]+) (LINE|CURVE|QCURVE|OFFCURVE|n/a)'
@@ -1628,11 +1612,13 @@ class GSNode(GSBase):
     )
     _parent = None
 
-    def __init__(self, position=(0, 0), nodetype=LINE, smooth=False, name=None):
+    def __init__(self, position=(0, 0), type=LINE, smooth=False, name=None, nodetype=None):
         self._userData = None
-        self.position = Point(position[0], position[1])
+        self._position = Point(position[0], position[1])
         self.smooth = smooth
-        self.type = nodetype
+        self.type = type
+        if nodetype is not None: # for backward compatibility
+            self.type = nodetype
         # Optimization: Points can number in the 10000s, don't access the userDataProxy
         # through `name` unless needed.
         if name is not None:
@@ -1651,6 +1637,16 @@ class GSNode(GSBase):
         lambda self, value: UserDataProxy(self).setter(value),
     )
 
+    @property
+    def position(self):
+        return self._position
+
+    @position.setter
+    def position(self, value):
+        if not isinstance(value, Point):
+            value = Point(value[0], value[1])
+        self._position = value
+
     @property
     def parent(self):
         return self._parent
@@ -1828,39 +1824,23 @@ class GSPath(GSBase):
 
         nodeCount = 0
         segmentCount = 0
-        nodes = list(self.nodes)
-        # Cycle node list until curve or line at end
-        cycled = False
-        for i, n in enumerate(nodes):
-            if n.type == "offcurve" or n.type == "line":
-                nodes = nodes[i:] + nodes[:i]
-                cycled = True
-                break
-        if not cycled:
-            return []
-
-        def wrap(i):
-            if i >= len(nodes):
-                i = i % len(nodes)
-            return i
-
-        while nodeCount < len(nodes):
+        while nodeCount < len(self.nodes):
             newSegment = segment()
             newSegment.parent = self
             newSegment.index = segmentCount
 
             if nodeCount == 0:
-                newSegment.appendNode(nodes[-1])
+                newSegment.appendNode(self.nodes[-1])
             else:
-                newSegment.appendNode(nodes[nodeCount - 1])
+                newSegment.appendNode(self.nodes[nodeCount - 1])
 
-            if nodes[nodeCount].type == "offcurve":
-                newSegment.appendNode(nodes[nodeCount])
-                newSegment.appendNode(nodes[wrap(nodeCount + 1)])
-                newSegment.appendNode(nodes[wrap(nodeCount + 2)])
+            if self.nodes[nodeCount].type == "offcurve":
+                newSegment.appendNode(self.nodes[nodeCount])
+                newSegment.appendNode(self.nodes[nodeCount + 1])
+                newSegment.appendNode(self.nodes[nodeCount + 2])
                 nodeCount += 3
-            elif nodes[nodeCount].type == "line":
-                newSegment.appendNode(nodes[nodeCount])
+            elif self.nodes[nodeCount].type == "line":
+                newSegment.appendNode(self.nodes[nodeCount])
                 nodeCount += 1
 
             self._segments.append(newSegment)
@@ -1951,15 +1931,19 @@ class GSPath(GSBase):
 
     # TODO
     def applyTransform(self, transformationMatrix):
+        raise NotImplementedError
+
+        # Using both skew values (>0.0) produces different results than Glyphs.
+        # Skewing just on of the two works.
+        # Needs more attention.
         assert len(transformationMatrix) == 6
         for node in self.nodes:
-            transformation = Affine(
-                transformationMatrix[0],
-                transformationMatrix[1],
-                transformationMatrix[4],
-                transformationMatrix[2],
-                transformationMatrix[3],
-                transformationMatrix[5],
+            transformation = (
+                Affine.translation(transformationMatrix[4], transformationMatrix[5])
+                * Affine.scale(transformationMatrix[0], transformationMatrix[3])
+                * Affine.shear(
+                    transformationMatrix[2] * 45.0, transformationMatrix[1] * 45.0
+                )
             )
             x, y = (node.position.x, node.position.y) * transformation
             node.position.x = x
@@ -2101,7 +2085,7 @@ class segment(list):
                 + (3 * mt * t * t * x2)
                 + (t * t * t * x3)
             )
-            if len(xvalues) > j:
+            if len(xvalues) > 0:
                 xvalues[j] = newxValue
             else:
                 xvalues.append(newxValue)
@@ -2111,7 +2095,7 @@ class segment(list):
                 + (3 * mt * t * t * y2)
                 + (t * t * t * y3)
             )
-            if len(yvalues) > j:
+            if len(yvalues) > 0:
                 yvalues[j] = newyValue
             else:
                 yvalues.append(newyValue)
@@ -3035,8 +3019,8 @@ class GSLayer(GSBase):
         "leftMetricsKey": None,
         "rightMetricsKey": None,
         "widthMetricsKey": None,
-        "vertWidth": None,
-        "vertOrigin": None,
+        "vertWidth": 0,
+        "vertOrigin": 0,
     }
     _wrapperKeysTranslate = {"guideLines": "guides", "background": "_background"}
     _keyOrder = (
@@ -3562,9 +3546,7 @@ class GSFont(GSBase):
         "DisplayStrings",
         "_classes",
         "_customParameters",
-        "_featurePrefixes",
         "_features",
-        "_customParameters",
         "_glyphs",
         "_instances",
         "_kerning",
@@ -3579,6 +3561,7 @@ class GSFont(GSBase):
         "disablesAutomaticAlignment",
         "disablesNiceNames",
         "familyName",
+        "featurePrefixes",
         "filepath",
         "grid",
         "gridSubDivisions",
@@ -3628,8 +3611,6 @@ class GSFont(GSBase):
     }
     _defaultsForName = {
         "classes": [],
-        "features": [],
-        "featurePrefixes": [],
         "customParameters": [],
         "disablesAutomaticAlignment": False,
         "disablesNiceNames": False,
@@ -3642,6 +3623,7 @@ class GSFont(GSBase):
 
     def __init__(self, path=None):
         self.DisplayStrings = ""
+        self._features = []
         self._glyphs = []
         self._instances = []
         self._masters = []
@@ -3649,8 +3631,6 @@ class GSFont(GSBase):
         self._versionMinor = 0
         self.appVersion = "895"  # minimum required version
         self.classes = copy.deepcopy(self._defaultsForName["classes"])
-        self.features = copy.deepcopy(self._defaultsForName["features"])
-        self.featurePrefixes = copy.deepcopy(self._defaultsForName["featurePrefixes"])
         self.copyright = ""
         self.customParameters = copy.deepcopy(self._defaultsForName["customParameters"])
         self.date = None
@@ -3661,6 +3641,7 @@ class GSFont(GSBase):
         ]
         self.disablesNiceNames = self._defaultsForName["disablesNiceNames"]
         self.familyName = "Unnamed font"
+        self.featurePrefixes = []
         self.filepath = None
         self.grid = self._defaultsForName["gridLength"]
         self.gridSubDivisions = self._defaultsForName["gridSubDivision"]
@@ -3730,6 +3711,18 @@ class GSFont(GSBase):
             ):
                 glyph._setupLayer(layer, layer.layerId)
 
+    @property
+    def features(self):
+        return self._features
+
+    @features.setter
+    def features(self, value):
+        # FIXME: (jany) why not use Proxy like every other attribute?
+        # FIXME: (jany) do the same for featurePrefixes?
+        self._features = value
+        for g in self._features:
+            g._parent = self
+
     masters = property(
         lambda self: FontFontMasterProxy(self),
         lambda self, value: FontFontMasterProxy(self).setter(value),
@@ -3757,16 +3750,6 @@ class GSFont(GSBase):
         lambda self, value: FontClassesProxy(self).setter(value),
     )
 
-    features = property(
-        lambda self: FontFeaturesProxy(self),
-        lambda self, value: FontFeaturesProxy(self).setter(value),
-    )
-
-    featurePrefixes = property(
-        lambda self: FontFeaturePrefixesProxy(self),
-        lambda self, value: FontFeaturePrefixesProxy(self).setter(value),
-    )
-
     customParameters = property(
         lambda self: CustomParametersProxy(self),
         lambda self, value: CustomParametersProxy(self).setter(value),


Test patch:
diff --git a/tests/builder/axes_test.py b/tests/builder/axes_test.py
index a3f49ccf..06423ac4 100644
--- a/tests/builder/axes_test.py
+++ b/tests/builder/axes_test.py
@@ -13,7 +13,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from copy import deepcopy
 import os.path
 
 import pytest
@@ -82,9 +81,6 @@ 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()
@@ -351,60 +347,8 @@ 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()
diff --git a/tests/builder/to_glyphs_test.py b/tests/builder/to_glyphs_test.py
index 0fc5b946..40e3267d 100644
--- a/tests/builder/to_glyphs_test.py
+++ b/tests/builder/to_glyphs_test.py
@@ -252,56 +252,6 @@ def test_glyph_color(ufo_module):
     assert ufo["b"].markColor == b.markColor
 
 
-def test_glyph_height_and_vertical_origin(ufo_module):
-    ufo = ufo_module.Font()
-    ufo.info.ascender = 1000
-    ufo.info.descender = -200
-    ufo.info.unitsPerEm = 1000
-
-    a = ufo.newGlyph("a")
-    a.height = 1024
-    a.verticalOrigin = 100
-    ufo.newGlyph("b")
-
-    font = to_glyphs([ufo])
-
-    assert font.glyphs["a"].layers[0].vertWidth == ufo["a"].height
-    assert font.glyphs["a"].layers[0].vertOrigin == 900
-    assert font.glyphs["b"].layers[0].vertWidth is None
-    assert font.glyphs["b"].layers[0].vertOrigin is None
-
-    (ufo,) = to_ufos(font)
-
-    assert ufo["a"].height == font.glyphs["a"].layers[0].vertWidth
-    assert ufo["a"].verticalOrigin == 100
-    assert ufo["b"].height == (ufo.info.ascender - ufo.info.descender)
-    assert ufo["b"].verticalOrigin == 1000
-
-
-def test_glyph_height_and_vertical_origin_trigger_vhea_metrics(ufo_module):
-    ufo = ufo_module.Font()
-    ufo.info.ascender = 1000
-    ufo.info.descender = -200
-    ufo.info.unitsPerEm = 1000
-
-    a = ufo.newGlyph("a")
-    a.height = 1024
-    a.verticalOrigin = 100
-
-    font = to_glyphs([ufo])
-
-    assert all(
-        "vhea" not in custom_param.name
-        for custom_param in font.masters[0].customParameters
-    )
-
-    (ufo,) = to_ufos(font)
-
-    assert ufo.info.openTypeVheaVertTypoAscender == 500
-    assert ufo.info.openTypeVheaVertTypoDescender == -500
-    assert ufo.info.openTypeVheaVertTypoLineGap == 1000
-
-
 def test_bad_ufo_date_format_in_glyph_lib(ufo_module):
     ufo = ufo_module.Font()
     a = ufo.newGlyph("a")
diff --git a/tests/classes_test.py b/tests/classes_test.py
index a9e70a14..d357b4a3 100755
--- a/tests/classes_test.py
+++ b/tests/classes_test.py
@@ -37,10 +37,8 @@ from glyphsLib.classes import (
     GSGuideLine,
     GSHint,
     GSNode,
-    GSPath,
     GSSmartComponentAxis,
     GSBackgroundImage,
-    segment,
     LayerComponentsProxy,
     LayerGuideLinesProxy,
     STEM,
@@ -1529,6 +1527,13 @@ class GSPathFromFileTest(GSObjectsTestCase):
         del path.nodes[-1]
         self.assertEqual(amount, len(path.nodes))
 
+    def test_node_position(self):
+        n = GSNode()
+        n.position = Point("{10, 10}")
+        self.assertEqual(n.position.x, 10)
+        n.position = (20,20)
+        self.assertEqual(n.position.x, 20)
+
     # TODO: GSPath.closed
 
     # bezierPath?
@@ -1536,29 +1541,6 @@ class GSPathFromFileTest(GSObjectsTestCase):
     # TODO:
     # addNodesAtExtremes()
     # applyTransform()
-    def test_applyTransform_translate(self):
-        pathCopy = copy.copy(self.path)
-        pathCopy.applyTransform((1, 0, 0, 1, 50, 25))
-        expected = ((402, 172), (402, 93), (364, 32), (262, 32))
-        for i, pt in enumerate(expected):
-            self.assertEqual(pathCopy.nodes[i].position.x, pt[0])
-            self.assertEqual(pathCopy.nodes[i].position.y, pt[1])
-
-    def test_applyTransform_translate_scale(self):
-        pathCopy = copy.copy(self.path)
-        pathCopy.applyTransform((0.9, 0, 0, 1.2, 50, 25))
-        expected = ((367, 201), (367, 107), (333, 33), (241, 33))
-        for i, pt in enumerate(expected):
-            self.assertAlmostEqual(pathCopy.nodes[i].position.x, pt[0], 0)
-            self.assertAlmostEqual(pathCopy.nodes[i].position.y, pt[1], 0)
-
-    def test_applyTransform_skew(self):
-        pathCopy = copy.copy(self.path)
-        pathCopy.applyTransform((1, 0.1, 0.2, 1, 0, 0))
-        expected = ((381, 182), (366, 103), (315, 38), (213, 28))
-        for i, pt in enumerate(expected):
-            self.assertAlmostEqual(pathCopy.nodes[i].position.x, pt[0], 0)
-            self.assertAlmostEqual(pathCopy.nodes[i].position.y, pt[1], 0)
 
     def test_direction(self):
         self.assertEqual(self.path.direction, -1)
@@ -1570,24 +1552,6 @@ class GSPathFromFileTest(GSObjectsTestCase):
         self.assertEqual(len(self.path.segments), 20)
         self.assertEqual(oldSegments[0].nodes[0], self.path.segments[0].nodes[0])
 
-    def test_segments_2(self):
-        p = GSPath()
-        p.nodes = [
-            GSNode((204,354),"curve"),
-            GSNode((198,353),"offcurve"),
-            GSNode((193,352),"offcurve"),
-            GSNode((183,352),"curve"),
-            GSNode((154,352),"offcurve"),
-            GSNode((123,364),"offcurve"),
-            GSNode((123,384),"curve"),
-            GSNode((123,403),"offcurve"),
-            GSNode((148,419),"offcurve"),
-            GSNode((167,419),"curve"),
-            GSNode((190,419),"offcurve"),
-            GSNode((204,397),"offcurve"),
-        ]
-        self.assertEqual(len(p.segments), 4)
-
     def test_bounds(self):
         bounds = self.path.bounds
         self.assertEqual(bounds.origin.x, 80)
@@ -1736,18 +1700,6 @@ class GSBackgroundLayerTest(unittest.TestCase):
             self.bg.foreground = GSLayer()
 
 
-class segmentTest(unittest.TestCase):
-    def test_bbox_bug(self):
-        seg = segment(
-            [Point(529, 223), Point(447, 456), Point(285, 177), Point(521, 367)]
-        )
-        bbox = seg.bbox()
-        self.assertAlmostEqual(bbox[0], 398.1222655016518)
-        self.assertAlmostEqual(bbox[1], 223)
-        self.assertAlmostEqual(bbox[2], 529)
-        self.assertAlmostEqual(bbox[3], 367)
-
-
 class FontGlyphsProxyTest(unittest.TestCase):
     def setUp(self):
         self.font = GSFont(TESTFILE_PATH)
@@ -1768,23 +1720,5 @@ class FontGlyphsProxyTest(unittest.TestCase):
             del self.font.glyphs[self.font]
 
 
-class FontClassesProxyTest(unittest.TestCase):
-    def setUp(self):
-        self.font = GSFont(TESTFILE_PATH)
-
-    def test_indxing_by_name(self):
-        assert "Languagesystems" in self.font.featurePrefixes
-        assert "c2sc_source" in self.font.classes
-        assert "aalt" in self.font.features
-
-        assert "XXXX" not in self.font.featurePrefixes
-        assert "XXXX" not in self.font.classes
-        assert "XXXX" not in self.font.features
-
-        assert self.font.featurePrefixes["Languagesystems"] in self.font.featurePrefixes
-        assert self.font.classes["c2sc_source"] in self.font.classes
-        assert self.font.features["aalt"] in self.font.features
-
-
 if __name__ == "__main__":
     unittest.main()
diff --git a/tests/writer_test.py b/tests/writer_test.py
index 2c956285..d7ba670d 100644
--- a/tests/writer_test.py
+++ b/tests/writer_test.py
@@ -87,7 +87,7 @@ class WriterTest(unittest.TestCase, test_helpers.AssertLinesEqual):
         font.features.append(f1)
         # featurePrefixes
         fp1 = classes.GSFeaturePrefix()
-        fp1.name = "FP1"
+        fp1 = "FP1"
         font.featurePrefixes.append(fp1)
         # copyright
         font.copyright = "Copyright Bob"
@@ -173,10 +173,7 @@ class WriterTest(unittest.TestCase, test_helpers.AssertLinesEqual):
             disablesNiceNames = 1;
             familyName = "Sans Rien";
             featurePrefixes = (
-            {
-            code = "";
-            name = FP1;
-            }
+            FP1
             );
             features = (
             {