search

cta-observatory / pycorsikaio

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

Added longitudinal fit parameters to 7.xx event end fields #37

Closed AndreaNegro953 closed 11 months ago

AndreaNegro953 commented 11 months ago

The 74100 file actually has all 0 in the space I am reading but that happens also for the longitudinal chi2 field that was already present in the origin version of pycorsikaio.

AndreaNegro953 commented 11 months ago

The test on the units of the event end seems to be the problem, need to figure out why since I had no errors on my machine.

maxnoe commented 11 months ago

@AndreaNegro953 The test should fail the same locally. The issue is that event_end_fields now is a dict mapping version numbers to the actual fields, not the fields directly anymore.

You need to adapt the test loop over the versions and check all of them.

AndreaNegro953 commented 11 months ago

@maxnoe Really strange then, I got only warnings about a deprecated thing of numpy, probably I messed something up when running pytest. Anyway I will fix the problem as soon as I can.

codecov[bot] commented 11 months ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (eef3b78) 95.89% compared to head (afa6a44) 95.48%. Report is 2 commits behind head on main.

Files Patch % Lines
corsikaio/subblocks/event_end.py 84.00% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #37 +/- ## ========================================== - Coverage 95.89% 95.48% -0.41% ========================================== Files 20 20 Lines 609 643 +34 ========================================== + Hits 584 614 +30 - Misses 25 29 +4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

maxnoe commented 11 months ago

Can you produce a small test file that contains values for these fields and check they are read correctly?

AndreaNegro953 commented 11 months ago

Of course, maybe for that I need a little bit more time.

maxnoe commented 11 months ago

@AndreaNegro953 I was able create a suitable test file.

Should I just add it here to your branch?

AndreaNegro953 commented 11 months ago

Sorry if I did not reply earlier Thank you for providing a test file, are there other things I should add to this pull request?

maxnoe commented 11 months ago

@AndreaNegro953 No, it looks good! Thanks a lot!

AndreaNegro953 commented 11 months ago

@maxnoe Me and @HealthyPear need to use the parameters added in this PR, would be possible to do a minor release to make this commit available?

maxnoe commented 11 months ago

@AndreaNegro953 Yes, I was planning to make a new release together with #39, should be out today

AndreaNegro953 commented 11 months ago

@maxnoe Thank you!

maxnoe commented 11 months ago

@AndreaNegro953 corsika 0.4 is on PyPI: https://pypi.org/project/corsikaio/, conda-forge package will come later today