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

Change how anchor name value is gotten #679

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

benkiel
Copy link
Member

@benkiel benkiel commented Jan 5, 2023

This should account for getting a value that doesn't exist in a math glyph, addressing #678

@benkiel benkiel requested a review from LettError January 5, 2023 16:45
@typemytype
Copy link
Member

the anchor.name attribute is optional according the spec, so is anchor.color, this could also be changed to anchor.get("color")

@benkiel
Copy link
Member Author

benkiel commented Jan 5, 2023

Agree, changed. Still not going to solve @LettError's problem, unfortunately

@justvanrossum
Copy link
Contributor

But look how toMathGlyph always sets name and color:

for anchor in self.anchors:
d = dict(
x=anchor.x,
y=anchor.y,
name=anchor.name,
identifier=anchor.identifier,
color=anchor.color
)
mathGlyph.anchors.append(d)

I don't quite understand what "the spec" (which spec?) has to do with this.

@benkiel
Copy link
Member Author

benkiel commented Jan 5, 2023

@LettError
Copy link
Member

Would like to test, but how do I get this code to run locally?

@justvanrossum
Copy link
Contributor

UFO Spec

Perhaps I'm nitpicking, but what I mean is, this is just an object implementation, and it can decide by itself whether it demands those attributes to exist or not. Perhaps it requires the "missing" attributes to be substituted by None. The spec specifies the format, not how objects should be implemented.

Also use get() for the guideline name and guideline color. There is a new, additional traceback relating to color in the issue discussion.
get the guideline color so we don't trip if the dict doesn't have it. Yet, this causes a different traceback
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

Successfully merging this pull request may close these issues.

4 participants