-
Notifications
You must be signed in to change notification settings - Fork 286
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
Support dynamic encodings #131
base: main
Are you sure you want to change the base?
Conversation
The setup is slow, so skipping it in normal pytest ways is ineffective. Now it's easier to run something like `pytest tests/core -k mytest` without incurring the slow benchmark setup.
b88b361
to
173bdd8
Compare
... and resolve new warnings
173bdd8
to
bf46cfd
Compare
This PR has a bit of housekeeping. The relevant bits are in the 2nd commit: 97d4771 |
try: | ||
return sedes.deserialize(obj) | ||
except ObjectDeserializationError: | ||
pass |
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.
What happens if all of the sedes options result in ObjectDeserializationError
. Won't that result in this returning None
?
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.
Yeah, sorry I "marked for review" a bit early. That's one of the TODOs listed above.
Demonstrate how to use dynamic structures in a test, and expose a
DynamicSerializable
.I suppose this can fix #112 if it gets merged. It's so small that I'm not sure it's necessary. Then again, I didn't realize it would be such a small addition until I wrote it.
TODO: