-
Notifications
You must be signed in to change notification settings - Fork 14
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
add support for de/serializing objects with json and msgpack #230
Conversation
Added some tests, I think this is now ready to review |
One thing this PR adds which I didn't mention in the description is the ability to pickle/unpickle ufoLib2 objects that are "lazy", i.e. they contain references to filesystem objects (via UFOReader or glifLib.GlyphSet) which aren't pickleable as such (because they hold a threading.RLock and that apparently is by design in pyfilesystem2). |
350ca77
to
0a8c32c
Compare
I have one question for typing enthusiast like @madig currently, since I add the So it's a bit inconvenient if one relies on typechecking to having to opt-out every time one uses these new de/serialization methods. One alternative is to not use the class/instance methods added by the E.g. instead of doing from ufoLib2 import Font
font = Font.json_load("MyFont.json") # type: ignore
font.json_dump("MyFont2.json") # type: ignore one could do instead: from pathlib import Path
import ufoLib2.serde.json
from ufoLib2 import Font
font = ufoLib2.serde.json.loads(Path("MyFont.json").read_bytes(), Font)
Path("MyFont2.json").write_bytes(ufoLib2.serde.json.dumps(font, indent=2)) It's a bit more verbose and indirect but makes mypy happy. |
similar to Font, DataSet, ImageSet
we currently don't have any attributes that are init=True and copyable=False, the only copyable=False is Font.path which is also init=False, so simply checking if init=False is enough
and in fact I had forgotten to decorate one of them (Info) ;)
Doing font.pickle_dumps() is exactly the same as doing pickle.dumps(font), same for Font.pickle_loads(s) vs pickle.loads(s), we don't use cattrs for pickling. So it's better to simply remove the extra code, pickling still works out of the box (even with lazy objects now).
in latest cattrs, GenConverter _is_ the normal Converter anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as far as I can see 👀
So one can do ufoLib2.serde.json.load('MyFont.json', ufoLib2.Font) in alternative to ufoLib2.Font.json_load('MyFont.json')
This adds methods for serializing / deserializing to/from JSON, MessagePack
and Pickleto all the ufoLib2.objects (Font, Glyph, etc.).It's mostly syntactic sugar around the ufoLib2.converters' structure/unstructure which provides convenience one-line methods for serialization to popular formats.
E.g.
or
Json serialization uses orjson on CPython as it's faster than built-in json. Optional dependencies (e.g. msgpack) are handled via extras.
It's just a proof-of-concept to solicit feedback, I simply drop it here in case it may be of use in our current effort to break up fontmake into parallel processes in order to speed it up.