robotools / fontParts

The replacement for RoboFab
MIT License
132 stars 44 forks source link

font.kerning.get() unnecessarily restricts types for default return value #723

Open BoldMonday opened 6 months ago

BoldMonday commented 6 months ago

font.kerning.get() allows for a default return value in case the pair is not present.

However, this afternoon I discovered that fontParts puts an unnecessary restriction on what types it allows for the default value. Where in former days of RoboFab the code below would happily work:

f = CurrentFont()
print(f.kerning.get(('A', 'Y'), 'no value found'))

Now I was suddenly confronted with this cryptic traceback:

  File "/Applications/RoboFont-4.2.app/Contents/Resources/lib/python3.7/fontParts/base/kerning.py", line 350, in get
  File "/Applications/RoboFont-4.2.app/Contents/Resources/lib/python3.7/fontParts/base/base.py", line 361, in get
  File "/Applications/RoboFont-4.2.app/Contents/Resources/lib/python3.7/fontParts/base/normalizers.py", line 152, in normalizeKerningValue
TypeError: Kerning value must be a int or a float, not str.

To me this seems an overly strict application of normalisation. Just like in standard Python dictionaries, default return values should be passed along as they are.

typemytype commented 6 months ago

according the docs it should either be a kern value or None

the fallback you can provide can also only be a int/float or None, nothing else...

I guess its best to test on None: print(c.kerning.get(("a", "b")) or "no value found")

https://fontparts.robotools.dev/en/stable/objectref/objects/kerning.html#fontParts.base.BaseKerning.find

Returns the value for the kerning pair. pair is a tuple of two Strings, and the returned values will either be Integer/Float or None if no pair was found.

BoldMonday commented 6 months ago

Thanks for the explanation and the help.

I guess its best to test on None: print(c.kerning.get(("a", "b")) or "no value found")

This will not work when the pair in question has a kerning value of 0 unfortunately. I solved it with a more elaborate if condition instead.