-
Notifications
You must be signed in to change notification settings - Fork 65
Change ArynWriter
to use new serialization approach
#1384
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
Conversation
…ase Document class
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.
I'd like to see our serialization format start with a magic number and version. I'm thinking 8 bytes like: Aryn
major16 minor16.
Overall, this seems like a sane way to use MessagePack, but I don't know enough about MessagePack to know if it'll satisfy our eventual needs.
I've now added magic bytes, a version header, and zero padding. |
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.
I still think we need to raise something other than ValueError in most cases. Generally, I think of ValueError as more of a programming error. I see agreement here: https://stackoverflow.com/questions/79590536/what-python-exception-should-i-raise-when-an-input-file-has-the-wrong-size
Done. |
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.
This looks great.
No description provided.