-
Notifications
You must be signed in to change notification settings - Fork 1
feat(utils): add binary reader and recorder for encoded states #152
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
bpapaspyros
left a comment
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.
Overall looks good, just a small question
|
What about the filename / names?
|
I don't mind too much (it is accurate enough). We could do
Yes, you are right, although if hadn't mentioned it I wouldn't have really thought much about it. Could we do perhaps |
996df02 to
90ed3bc
Compare
eeberhard
left a comment
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.
Functionality is good, but I think if we are writing encoded state to file as encoded binary representation, then specifically calling them encoded state recordings would be most correct. I would then also call the whole module encoded_state_recorder, as binary_io is just a bit vague.
eeberhard
left a comment
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 like it like this!
Co-authored-by: Enrico Eberhard <32450951+eeberhard@users.noreply.github.com>
Description
In this PR I'm adding utilities to write and read recorded files that contain encoded states along with the timestamp of when the data was recorded.
Review guidelines
Estimated Time of Review: 5 minutes
Checklist before merging: