PathPen should raise instead of silently ignoring components

[khaledhosny]

Currently, PathPen.addCompnent() silently ignores the component, which will give unexpected results if one is not aware of this. It should raise instead so client code can catch it and decide what it needs to do.

[anthrotype]

Maybe.. How about a logging.warning instead? It feels a bit too harsh to raise, having skipped them throughout all this time.

[khaledhosny]

In my case I was drawing glyphs from glyf table with it to remove overlaps, and it silently gave me blanks for composites. I ended up checking for composites and skipping them, but try: except: would have been more Pythonic.

[behdad]

I'm in for raising. Just because it was broken so far isn't justification enough for keeping it that way.

We're currently discussing adding pens to HarfBuzz, and I was considering abort()ing in such situations as well. It's just too silent of an error to let go when we can stop.

[typemytype]

remove overlap happens on contours not on components, if you wish to apply remove overlap on components, you need to decompose before.

I guess its good not to raise as you can apply a remove overlap on contours and still keep the components around...

[anthrotype]

I guess its good not to raise as you can apply a remove overlap on contours and still keep the components around...

A glyph in TrueType is either simple or composite, not both like in UFO where that consideration may make sense.

I'm ok to raise NotImplementedError in PathPen.addComponent. In ufo2ft we only union the contours (after components have been decomposed) so it would not affect us. But it might help situations like Khaled found himself.

[typemytype]

True, but I assume the pen can also be used in a ufo world or any other tools supporting the pen protocol.

[anthrotype]

definitely, it can and it is used in UFO world.

even booleanOperations pen raises, see https://github.com/typemytype/booleanOperations/blob/d9e70775c4dd4d4ada5239880b8e34f5addb1cec/Lib/booleanOperations/flatten.py#L355-L356

[anthrotype]

when you have a mixed UFO glyph with contours-components, you can always only apply the operations only on the glyph.contours (which support the pen API like the parent Glyph).

[typemytype]

the pen you mention is an internal pen, where only contours must be used.

Booleanoperation has glyph like objects with public pens and support the combo of contours and components. It restores the components with merged contours afterwards.

see https://github.com/typemytype/booleanOperations/blob/d9e70775c4dd4d4ada5239880b8e34f5addb1cec/Lib/booleanOperations/booleanGlyph.py#L59-L60

(I just advocating for the long period, aka a design process, before a final drawing is done, where multiple data types of outlines are combined)

[khaledhosny]

pathops is rather low level library, if someone needs something more higher level, then pen can be subclassed and amended or skiping components in the function drawing to it, or any other solution. Silently ignoring components is too bad, printing a warning is also bad (slightly less than being silent). Raising is the only acceptable way to handle a missing feature like this, this is what Python exceptions are for.

[anthrotype]

I have a better idea. If the PathPen is provided with a glyphSet parameter in its constructor, it will use that in addComponent to decompose the components. Otherwise we let it raise an exception. The glyphSet parameter will be None by default, so the default behavior will change to raise exception (instead of silently ignoring components like now). I'll also add a glyphSet parameter to the Path.getPen() method for convenience. I find myself needing this in https://github.com/fonttools/fonttools/pull/2068

[khaledhosny]

Thanks @anthrotype!