project8 / hercules

Python package for scripting our simulation workflows
1 stars 0 forks source link

Feature/egg reader improvements #10

Closed charstnut closed 3 years ago

charstnut commented 3 years ago

I've rewritten the eggreader so that it now reads the data on a stream by stream basis. The loaded timeseries and FFTs are structured according to Monarch Doc.

charstnut commented 3 years ago

Noted. This will be implemented in a few days.

charstnut commented 3 years ago

There are some extra work I've added.. Basically I implemented the unittest framework for the test folder and modifies the existing tests.

MCFlowMace commented 3 years ago

Thanks @charstnut, looks good to me. Also the addition of proper unittesting is nice. I made a small change regarding more efficient memory layout for the final data. And I removed the part that added the module to the sys path in the tests. I think it is better to run the tests with the installed module. That way tests are also a check for a proper installation. If you agree with those changes we can merge.

Finally, I would like to suggest for the future that you use the pathlib library in place of calls to os (like os.path.join, or os.mkdir). It is much nicer https://treyhunner.com/2018/12/why-you-should-be-using-pathlib/ .

charstnut commented 3 years ago

Thanks for the review and changes @MCFlowMace! I will take a quick look tomorrow soon and merge the branch. Also sure I will switch to pathlib whenever possible.