-
Notifications
You must be signed in to change notification settings - Fork 2
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 for thin type file structure #42
Conversation
Hi, Thanks @oven8, this looks mostly good. I think we mainly need a smallish test file with the thin option enabled to make sure this keeps working. Some of the duplications can be removed and the test paths change needs to be reverted. |
I'll add an update with the changes you recommended as well as a small test module. |
I have added thin (Cherenkov) binary data file and modified some of the test modules. Let me know if more changes are required. |
@oven8 This looks mostly good, thanks! The one concern I have is that the particle test file at 20 MB is quite large for a test file. Could you maybe create a smaller one, the Cherenkov file is fine size-wise! |
I've swapped the thin test file with a smaller one (previously there were 200 events now 5). Let me know if this is okay. |
@oven8 I am sorry this took so long, looks good now. Feel free to ping me more often next time. I am going to squash the commits here to make sure the larger test files that were here in intermediate commits don't land in the project history. |
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.
Thanks a lot for this contribution!
Support for thin type file structure
Thanks for all the help! It was nice collaborating with you! |
I've added modifications to support thin type file structures. The main differences are noted below:
One important change: I had to remove the classmethod decorator over the parse_data_blocks method in the CorsikaFile class.
The code passed the existing pytest modules. I have not added new modules to test for thin binary data files. However I've separately tested with thin data and things seems to be working fine so far. I might modify the test files to add new modules to check for the changes.