cta-observatory / pycorsikaio

Python reader for CORSIKA binary file format
MIT License
9 stars 2 forks source link

Fix iter blocks #34

Closed maxnoe closed 1 year ago

maxnoe commented 1 year ago

Iter blocks didn't have an exit condition...

This only worked because the CorsikaFile stops iterating the generator when it sees the RUNE block, but a broken file would have ended up in an infinite loop.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 93.18% and project coverage change: -0.27% :warning:

Comparison is base (1030e7e) 96.16% compared to head (f42baa8) 95.89%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #34 +/- ## ========================================== - Coverage 96.16% 95.89% -0.27% ========================================== Files 20 20 Lines 573 609 +36 ========================================== + Hits 551 584 +33 - Misses 22 25 +3 ``` | [Files Changed](https://app.codecov.io/gh/cta-observatory/pycorsikaio/pull/34?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory) | Coverage Δ | | |---|---|---| | [corsikaio/file.py](https://app.codecov.io/gh/cta-observatory/pycorsikaio/pull/34?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory#diff-Y29yc2lrYWlvL2ZpbGUucHk=) | `95.41% <75.00%> (-2.13%)` | :arrow_down: | | [corsikaio/io.py](https://app.codecov.io/gh/cta-observatory/pycorsikaio/pull/34?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory#diff-Y29yc2lrYWlvL2lvLnB5) | `92.75% <100.00%> (+0.81%)` | :arrow_up: | | [tests/test\_file.py](https://app.codecov.io/gh/cta-observatory/pycorsikaio/pull/34?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory#diff-dGVzdHMvdGVzdF9maWxlLnB5) | `100.00% <100.00%> (ø)` | | | [tests/test\_io.py](https://app.codecov.io/gh/cta-observatory/pycorsikaio/pull/34?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory#diff-dGVzdHMvdGVzdF9pby5weQ==) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.

:loudspeaker: Have feedback on the report? Share it here.

maxnoe commented 1 year ago

The changes look fine but it seems you are not testing almost any of the newly added exceptions raised. If I understood correctly, the new test function test_iter_blocks_all tests that with a normal (but empty) file, you get the correct number of blocks, but I am not sure if it would test the iter exists.

Yes, true, will add some tests using arbitrary truncation. The most important one though is the newly added test, which before these fixes here resulted in an infinite loop.

maxnoe commented 1 year ago

@orelgueta Ok, added some better tests.

Didn't manage to get all error conditions though, but I think it is not really important, they are all basically the same and the effort to create a file where the truncation is on the edge of a super-block inside of one event seems not warranted