Fix bug when µMUX data has no number of rows stored in LJH file #300
Conversation
| self.column_number = None | ||
| self.subframe_divisions = None | ||
| self.subframe_offset = None | ||
| self.number_of_rows = 1 |
There was a problem hiding this comment.
This set of changes seems totally uneccesary and actually bad. We found this error because mass crashed when something went wrong, by giving default values instead of None values to all these parameters, we increase the likelyhood mass will do something silently wrong instead of crashing. These changes aren't needed to fix the bug at hand.
| if "Abaco" in self.source: | ||
| # The external trigger file can override this, but assume 64 divisions at first. | ||
| default_divisions = 64 | ||
| default_offset = 0 | ||
| self.subframe_divisions = int(header_dict.get(b"Subframe divisions", default_divisions)) | ||
| self.subframe_offset = int(header_dict.get(b"Subframe offset", default_offset)) | ||
| if self.subframe_divisions <= 0: |
There was a problem hiding this comment.
This is a redundant fix, if the above fix works, this isn't necessary. So future readers will have a hard time understanding why this exists and it will make reading the code harder without adding any value.
There was a problem hiding this comment.
The issue title says that the issue happane when no number of rows is stored in the LJH file. But we have a line should make number_of_rows=1 in that case, which should prevent the divide by zero. So I also don't understand from this how the issue ever happens.
| default_offset = self.row_number | ||
| default_divisions = self.number_of_rows |
There was a problem hiding this comment.
I believe this is the only place we should have a fix, and it should have a comment explaining why it works the way it does, and probably print a warning about what is wrong that is making it need the fix. Perhaps "warning: malformed ljh filed with number_of_rows = 0 detected, doing x instead".
No description provided.