Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

anchor and guideline may not have specific keys in dict #678

Open
LettError opened this issue Jan 5, 2023 · 13 comments
Open

anchor and guideline may not have specific keys in dict #678

LettError opened this issue Jan 5, 2023 · 13 comments

Comments

@LettError
Copy link
Member

LettError commented Jan 5, 2023

After some mathGlyphing around in RF Version 4.4b (build 2212022233) I end up here as part of a fromMathGlyph():

name=anchor["name"],

Traceback (most recent call last):
  File "testRefactor_RF.py", line 64, in <module>
  File "/Users/erik/code/fontParts/Lib/fontParts/base/glyph.py", line 1669, in fromMathGlyph
  File "lib/fontObjects/fontPartsWrappers.pyc", line 87, in wrapper
  File "/Users/erik/code/fontParts/Lib/fontParts/base/glyph.py", line 1697, in _fromMathGlyph
KeyError: 'name'

Mysteriously, the anchors of the mathGlyph appear to have names:

[{'name': 'top', 'identifier': None, 'x': 235.72533333333334, 'y': 854.5736666666667, 'color': None}, {'name': 'bottom', 'identifier': None, 'x': 258.90666666666664, 'y': 0.0, 'color': None}]

Maybe something in RF's fontPartsWrappers?

@benkiel
Copy link
Member

benkiel commented Jan 5, 2023

@LettError Must be, as fontParts doesn't implement _get_name or _set_name in base

@benkiel
Copy link
Member

benkiel commented Jan 5, 2023

But, maybe we should change this up to check if there is a name, looking into it

@LettError
Copy link
Member Author

It's looking for a name in a dict?

@benkiel
Copy link
Member

benkiel commented Jan 5, 2023

It's looking for the anchor name from the math glyph object. I think that changing it to a get makes sense so that if there is no anchor name, it gets set to None.

@justvanrossum
Copy link
Contributor

But Erik mentioned that the anchors do have names, so something appears off. Not failing when there's no name may not solve this actual problem.

@benkiel
Copy link
Member

benkiel commented Jan 5, 2023

@justvanrossum right, the above solves the first issue, but yes, not sure about what fontPartsWrappers is doing

@benkiel
Copy link
Member

benkiel commented Jan 5, 2023

Looking through all the code, I'm not seeing how that name is getting lost

@benkiel
Copy link
Member

benkiel commented Jan 5, 2023

But, yes, @justvanrossum, you're right, the above solves the key error for the optional name and now color, but not the issue @LettError is running into. I looked at Defcon, which is what RF is wrapping, and I don't see how the name is getting lost there, looked at RF code, not seeing it there, etc. So I'm also at a bit of a loss.

@justvanrossum
Copy link
Contributor

Mysteriously, the anchors of the mathGlyph appear to have names

The traceback shows this is not the case, though. Did you find out you have anchors without a name after all? If not, I would suggest looking into that first. If yes, then #679 is likely the right solution.

I feel the analysis of this bug is incomplete, and therefore the proposed fix could be premature. Especially since toMathGlyph() does guarantee for anchor names to exist.

@LettError
Copy link
Member Author

LettError commented Jan 5, 2023

Analysis of the bug is indeed not complete, it was not possible to make a neatly isolated testable thing. However, some progress.

Note 1: maybe I misreported anchor rather than guideline. I don't know whether I reported it wrong, or whether I tested with a different glyph with different anchors. I understand this is not ideal, but there is progress anyway!

Note 2: There definitely is an issue with guideline, it may also be anchor. But as that is already changed in this branch and that specific traceback did not come back so I will just leave the fix in anchor.

New issue 1: guideline["name"] -> guideline.get("name") # ok that works, phew
New issue 2: guideline["color"] -> guideline.get("color") # ah, new traceback, see below

Changes made in this branch:

color=guideline.get("color") # XX

MathGlyph guidelines, no names, and only 1 with a color.

{'x': 171.16424854606674, 'y': 398.3774716865626, 'angle': 39.17460681282844, 'identifier': '4jjGjAobNR'}
{'x': 296.0430976430976, 'y': 196.8342822161004, 'angle': 214.52107759503738, 'identifier': 'nFXtuqy0Zx'}
{'x': 224.6831955922865, 'y': 415.5753902662994, 'angle': 39.19414618573358, 'identifier': 'IO6VxBZgbG'}
{'x': 415.8800122436486, 'y': 613.9582491582493, 'angle': 273.92069015079494, 'color': '0.999,0.001,0.001,0.999', 'identifier': 'sieX9anfuL'}

Running the code in this branch, with these guidelines gives a new traceback centered on color:

Traceback (most recent call last):
  File "testRefactor_RF.py", line 69, in <module>
  File "/Users/erik/code/fontParts/Lib/fontParts/base/glyph.py", line 1680, in fromMathGlyph
  File "lib/fontObjects/fontPartsWrappers.pyc", line 87, in wrapper
  File "/Users/erik/code/fontParts/Lib/fontParts/base/glyph.py", line 1705, in _fromMathGlyph
  File "/Users/erik/code/fontParts/Lib/fontParts/base/glyph.py", line 1389, in appendGuideline
  File "/Users/erik/code/fontParts/Lib/fontParts/base/normalizers.py", line 921, in normalizeColor
TypeError: Colors must be tuple instances, not Color.

So, the name issues appear gone at least :)

@LettError LettError changed the title anchor might not have name here anchor and guideline may not have specific keys in dict Jan 5, 2023
@benkiel
Copy link
Member

benkiel commented Jan 5, 2023

Ah, that error is the normalizer being grumpy, we can fix that!

@benkiel
Copy link
Member

benkiel commented Jan 7, 2023

I guess the thing to do in the normalizer would be to transform the Color object to a tuple

@benkiel
Copy link
Member

benkiel commented Jan 7, 2023

Ok, that's weird as the normalizer allows Color, at least a fontParts.base.Color

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants