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

make Glyph.name optional for compatibilty with defcon #51

Merged
merged 5 commits into from
Feb 20, 2020

Conversation

anthrotype
Copy link
Member

as @typemytype wrote in googlefonts/ufo2ft#363 (comment)

Its maybe not a good plan to make the api between the ufoLib2 and defcon so different. According the spec a glyph name is not required, so on init of a glyph object the name should not be a requirement. But ufoLib2 makes it a required argument, I think this is wrong... and makes packages like ufo2ft complex where it should not be...

this PR makes Glyph.name default to None (e.g. for Glyphs without a parent layer).
As soon as a name-less glyph is added to a Layer object, it gets a name defined by its key in the layer underlying dict.

This is to make it easier to use ufoLib2 as drop-in replacement for defcon API.
defcon.Glyph.name defaults to None, and it can't be set from init constructor.
… unique)

the same Glyph object can't be added twice because the name attribute is also its key and it must be unique.
this copies and overwrites by default.

addGlyph (unique to ufoLib2) does not copy and does not overwrite.
@codecov-io
Copy link

codecov-io commented Feb 20, 2020

Codecov Report

Merging #51 into master will increase coverage by 3.4%.
The diff coverage is 95.45%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #51     +/-   ##
=========================================
+ Coverage   71.95%   75.36%   +3.4%     
=========================================
  Files          19       19             
  Lines        1077     1108     +31     
  Branches      160      172     +12     
=========================================
+ Hits          775      835     +60     
+ Misses        258      230     -28     
+ Partials       44       43      -1
Impacted Files Coverage Δ
src/ufoLib2/objects/glyph.py 69.44% <100%> (+2.08%) ⬆️
src/ufoLib2/objects/layer.py 85.61% <95.23%> (+26.48%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f56f62...9e8296e. Read the comment docs.

src/ufoLib2/objects/layer.py Show resolved Hide resolved
tests/objects/test_layer.py Show resolved Hide resolved
@anthrotype anthrotype merged commit c16a568 into master Feb 20, 2020
@anthrotype anthrotype deleted the optional-glyph-name branch February 20, 2020 20:00
@typemytype
Copy link

thanks!

@anthrotype
Copy link
Member Author

@typemytype do let us know if you find other API incompatibilities that we could fix, thanks

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