Skip to content
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

GSB stream reader's fill_value doesn't do anything #105

Closed
cczhu opened this issue Dec 21, 2017 · 4 comments
Closed

GSB stream reader's fill_value doesn't do anything #105

cczhu opened this issue Dec 21, 2017 · 4 comments

Comments

@cczhu
Copy link
Contributor

cczhu commented Dec 21, 2017

Following on #101, fill_value in GSB doesn't do anything. This makes sense since GSB has no built-in way to flag for invalid data. I'm not sure why the option even exists, though - should we just get rid of it?

@mhvk
Copy link
Owner

mhvk commented Dec 21, 2017

@cczhu - but GSB can have missing data blocks, which would be noticed from the timestamp file. Still, since currently we do not do anything with it, I guess we might as well remove it for now.

@cczhu
Copy link
Contributor Author

cczhu commented Dec 21, 2017

@mhvk that's true, but since payload-timestamp connections are inferred rather than embedded within the GSB file, is it even safe to keep reading after hitting a missing timestamp? It could be that the payload kept writing, or was partly written. I suppose we could do a complete file integrity check at the beginning to make sure the payload file sizes add up to the number of header lines, and then assume missing data only if everything does check out (which still leaves the possibility for some forms of time offsets to go unchecked)?

@mhvk
Copy link
Owner

mhvk commented Mar 14, 2018

"Needs clarification" since I think there is a decision to be made here: my current sense is that fill_value should be passed in at file opening time rather than for reading, since it is very unlikely one would want to change it half-way through (see #146). An advantage of that method is that fh.read() starts to become completely independent of what the underlying stream is. If we go that route, then for gsb the conclusion is simply to remove fill_value from its read method.

@mhvk
Copy link
Owner

mhvk commented Mar 22, 2018

fixed by #163 (by removing the fill_value for now).

@mhvk mhvk closed this as completed Mar 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants