Examine potential problems with Glyphs compatibility.

[typesupply]

This issue is a continuation of the comments in #398. @schriftgestalt, could you post your fontParts wrapper code as a file attached to a comment? I'll look into the issues you mentioned.

[schriftgestalt]

I’m looking into this again. I started with the test script from fontshell. Setting up some of the classMappings (some will be tricky).

from fontParts.test import testEnvironment
classMapping = dict(
    font=GSFont,
#    info=RInfo,
#    groups=RGroups,
#    kerning=RKerning,
#    features=RFeatures,
    layer=GSLayer,
    glyph=GSGlyph,
    contour=GSPath,
    segment=GSPathSegment,
#    bPoint=FSTestBPoint,
    point=GSNode,
    anchor=GSAnchor,
    component=GSComponent,
    image=GSBackgroundImage,
#    lib=RLib,
    guideline=GSGuide,
)

def fontshellObjectGenerator(cls):
    unrequested = []
    obj = classMapping[cls]()
    return obj, unrequested

testEnvironment(fontshellObjectGenerator, inApp=True, verbosity=2)

I was told that fontParts is more an API promise than an implementation and that I can put my own classes into the classMappings as long as they follow the API. But that doesn’t seem to be the case. I’m trying to use my internal objects directly and see if I can teach them the fontParts API (instead of wrapping them in python objects. I had that with the robofab implementation but I managed to remove the wrapper for some objects). The problem is that I get a lot errors like this:

======================================================================
ERROR: test_normalizeAnchor_valid (fontParts.test.test_normalizers.TestNormalizers)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_normalizers.py", line 981, in test_normalizeAnchor_valid
    result = normalizers.normalizeAnchor(anchor)
  File "normalizers.py", line 661, in normalizeAnchor
    return normalizeInternalObjectType(value, BaseAnchor, "Anchor")
  File "normalizers.py", line 727, in normalizeInternalObjectType
    raise TypeError("%s must be a %s instance, not %s."
TypeError: Anchor must be a Anchor instance, not GSAnchor.

It expects an baseclass that I can’t provide.

[benkiel]

@schriftgestalt do you have this sketch up someplace I can take a look? It looks like there may just be a simple thing with how you've got it set up, but I'm not sure without seeing all the code. The text.py file, that you have a section of above, should map things so that you don't get errors like the above, so it seems like you're doing the right thing from what I can see—so there may just be something that needs to be tweaked slightly.

[schriftgestalt]

It checks explicitly if the object is a BaseAnchor:

 # normalizers.py line:661
    from fontParts.base.anchor import BaseAnchor
    return normalizeInternalObjectType(value, BaseAnchor, "Anchor")

[typesupply]

It checks explicitly if the object is a BaseAnchor:

Aha, I see. Are you using any of the functions from fontParts.base.normalizers? I'm guessing not since those also reference the BaseThing objects. If you aren't using the normalizer functions, the test isn't applicable. We should make it possible to skip those tests.

@benkiel If Georg confirms my hunch, could we solve this by adding a testNormalizers argument to testEnvironment? When True (the default), test_normalizers would be appended to testModules.

[schriftgestalt]

I don’t know anything about the normalizers. I’m just getting started to understand this. I’ll disable the test for now. Thanks

[typesupply]

Yeah, disable that for now and hopefully it'll give you better results.

The normalizers are used by the base classes to ensure that consistent value types are passed to the environment implementations. The idea is that anything that inherits from BaseThing can be sure that it's only going to get int values when int values are the only thing allowed and, therefore, the environment doesn't have to do any sanity checking on the incoming values. But, this is only applicable to environments that inherit from the base classes, so we should make this test optional.

[benkiel]

Agree, adding the switch for the tests is a good idea, @schriftgestalt I'll do that.

[benkiel]

Ok, #502 should do it, @schriftgestalt then you can just turn all those tests off. Waiting to be sure the checks pass before merging.

[benkiel]

@schriftgestalt and merged. You can now just set testNormalizers=True in testEnvironment and those tests won't run

[benkiel]

@schriftgestalt a possible dumb question: from your issues and the above it seems like you aren't wrapping objects so you have to re-implement all of FontParts: is there a strong reason for doing so? It seems to me like a ton of extra work, but I may be misunderstanding what you're doing or not seeing a reason for doing things that way.

[schriftgestalt]

I started with wrapping stuff in the roboFab implementation. That didn't help much as I still have a tone of glue code and the base implementation gets in the way here and there. For the fontParts implementation, I use a similar technique as for the internal scripting API.

so giving the contour class (called GSPath in Glyphs) a .points property is easily done like this: GSPath.points = GSPath.nodes.

There are places where that leads to conflicts (in Glyphs, .bounds return a NSRect, in fontPart a (minx, miny, maxx, maxy). I have to see what to do with those.

[schriftgestalt]

And I hope I’m not too annoying with all those issues and question. I’m just working on this intensely right now.

[benkiel]

Ah, yeah, I can only imagine that trying to use the roboFab implementation to wrap fontParts would be crazy making. I'm only suggesting that you might look at just wrapping your objects to fontParts, but I think that's what you're doing now, but the way I'd do it is to make a wrapper (GFont, GPath, etc) for your GSFont, GSPath objects. If you look at what fontShell does, it has a base object (https://github.com/robotools/fontParts/blob/master/Lib/fontParts/fontshell/base.py) to handle the wrapping, and then for each object it does:

import GSPath
from fontParts.base import BaseContour
from GS.fontParts.base import GBaseObject

class GContour(GBaseObject, BaseContour):

    wrapClass = GSPath

Does that make sense?

Totally understand needing to ask where things don't make sense—keep asking away. It is good to have more eyes on this to see edge cases and things that don't make sense. Just know that right now my time is somewhat limited, I'm trying to reply as quickly as I can as I know you're working on this and it would be fantastic to see in Glyphs.

[benkiel]

There are places where that leads to conflicts (in Glyphs, .bounds return a NSRect, in fontPart a (minx, miny, maxx, maxy). I have to see what to do with those.

Couldn't you just return (NSMinX, NSMinY, NSMaxX, NSMaxY) for that NSRect?

[schriftgestalt]

Ah, yeah, I can only imagine that trying to use the roboFab implementation to wrap fontParts would be crazy making.

I didn’t mean that. I meant my experience implementing that wrapper, years ago. But I turned out that I could salvage the Info implementation.

Couldn't you just return (NSMinX, NSMinY, NSMaxX, NSMaxY) for that NSRect?

I would need to change the implementation for both, the internal API and fontParts.

[benkiel]

I guess that's where having the wrapper could be helpful, so that you can pull data from your GS objects and just give them to fontParts the way it expects, but you know how this works for Glyphs way better than I do, so apologies for very likely not quite understanding. If what you're working on is public, I'd be happy to look to understand a bit more (I looked in your schriftgestalt repositories but didn't see anything, apologies if i've missed it)

[schriftgestalt]

There are some edge cases like this in the current approach and a big bag of problems in the other.

The current python API is here: https://github.com/schriftgestalt/GlyphsSDK/blob/master/ObjectWrapper/GlyphsApp/__init__.py

the robofab wrapper is here: https://github.com/schriftgestalt/Glyphs-Scripts/blob/master/objectsGS.py

I’ll put the fontParts wrapper in the GlyphsSDK repo once I’ll gone through the tests.

[schriftgestalt]

I’ll add stuff that I stumble across:

• bPoint is not fitting at all with the Glyphs object model.
• I don't see a way to implement font.layers (except maybe the background layer) *1
• The test def test_save_fileStructure(self): seems to test private API? No mentioning of fileStructure in the docs.
• There are tests like this. Wouldn‘t is be better if it returned on AttributeError, too? Because those I get.

  def test_selected_false(self):
      font = self.getFont_glyphs()
      try:
          font.selected = False
      except NotImplementedError:
          return
      self.assertEqual(
          font.selected,
          False
      )

• I’m not sure about how to implement the info object as an independent thing (to work with the current test environment. It needs to be a proxy that funnels the info to the correct spot as the info is stored in various places inside Glyphs (font, masters, native properties, custom parameters). Is someone ever going to instantiate an info object in a script?

1) I still don't get the concept. This seems to me as if I could ask the bookshelf to give me the third pages of all books. It (seems) to allow a layer "XYZ" that contains a glyph "A" but not have a default layer for that glyph name?

[madig]

If you don't mind me interjecting, I think completely wrapping fontParts (UFO data model by another name) around the Glyphs data model of the world is probably going to be leaking at the seams. We have the same problem in glyphsLib, and only with a Designspace do we get a semblance of model compatibility. I suspect the same might be happening in FontLab 7.x.

Maybe it would make sense to define a profile with just a subset of objects, e.g. equating Font with a Glyphs master and leaving out Info and Features, which in Glyphs are attached elsewhere. The Font-Layer problem could maybe be hamfistedly solved by having Glyphs union the glyph names of all layers and creating glyph objects for them as necessary, so that it never gets into an invalid state. This would lead to some ghostly interactions behind the user's back, but maybe it's good enough? That way, users could script at least the parts they are probably most interested in.

[schriftgestalt]

I thought about using the master as the "Font". I already do a similar thing with the glyph/layer as that I use the layer as "glyph". That works until someone asks an element (path, component…) for its .glyphs or .layer. It will get things that it will not be happy with.

The font.info works fine, only info without a font object doesn't make sense, but that is probably only a problem with the tests.