search

daltonmaag / statmake

Generate STAT tables for variable fonts from .stylespace files
MIT License
39 stars 11 forks source link

Additional Locations is required #35

Open aaronbell opened 4 years ago

aaronbell commented 4 years ago

At current the code is set up to require additional locations to be defined when calling statmake.lib.apply_stylespace_to_variable_font, even if one does not have additional locations to define. Additionally, it looks like the _sanity_check code and _generate_builder_data also assume the presence of additional defined locations.

I went through and added if additional_locations is not None: to my copy of lib.py so now it works ok! Figure you might want to implement it a different way, so thought I'd just file a bug.

madig commented 4 years ago

I did that so I didn't need to deal with Nones, which I find annoying. Setting {} as the default argument does no good in Python. Just add an empty {} to the call: https://github.com/daltonmaag/statmake/blob/v0.3.0/src/statmake/cli.py#L57

aaronbell commented 4 years ago

Fair enough. I still think it would be better to have the additional_locations set as optional rather than required (and provide a default state of {} if not explicitly set). That way one doesn't even have to include empty braces if it is unnecessary (which I find a bit annoying 😆 )

madig commented 4 years ago

Mh.. well Ok, I guess it won't hurt to add a is None check in apply_stylespace_to_variable_font.