-
-
Notifications
You must be signed in to change notification settings - Fork 1
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
PR on bin file iterator and first event visualization example #253
Conversation
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.
Just one comment - can you check it?
examples/visualize_2d.py
Outdated
t_current += t_window | ||
i += 1 | ||
|
||
# TODO: When using a for loop the event iterator stays in an infinite loop |
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 is because IteratorBin
class does not have StopIteration
.
Iterator needs to tell the condition for the termination. Something like: https://github.com/shiba24/event-vision-library/blob/main/src/evlib/codec/fileformat/text.py#L37
Can you have a way to tell if there is any more event to read?
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.
In this case I read all the .bin file in one time (which may not be the best practice for huge files), so the iterator just "iterates" a single time.
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.
OK. My design intention is IteratorAccess
is for any filesize (can be large), so the loader should not assume it to be reasonably small. For smaller size, I found it more useful to use BlockAccess
like hdf5 loader (assume to load all contents at once). At least this should work for timestamps.
But let's proceed as it is. Thanks for the PR.
This PR contains: