search

cta-observatory / ctapipe_io_nectarcam

ctapipe plugin for reading nectarcam files
BSD 3-Clause "New" or "Revised" License
2 stars 11 forks source link

`ctapipe_io_nectarcam` fails when called with `ctapipe` #42

Closed jlenain closed 1 year ago

jlenain commented 1 year ago

This PR should fix issue #41.

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (cfc5bdb) 86.12% compared to head (b678680) 88.51%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #42 +/- ## ========================================== + Coverage 86.12% 88.51% +2.39% ========================================== Files 7 7 Lines 627 627 ========================================== + Hits 540 555 +15 + Misses 87 72 -15 ``` | [Files](https://app.codecov.io/gh/cta-observatory/ctapipe_io_nectarcam/pull/42?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory) | Coverage Δ | | |---|---|---| | [src/ctapipe\_io\_nectarcam/\_\_init\_\_.py](https://app.codecov.io/gh/cta-observatory/ctapipe_io_nectarcam/pull/42?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cta-observatory#diff-c3JjL2N0YXBpcGVfaW9fbmVjdGFyY2FtL19faW5pdF9fLnB5) | `85.56% <100.00%> (+3.23%)` | :arrow_up: |

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

jlenain commented 1 year ago

@tibaldo , @vmarandon , I do not understand exactly why the CI fails while in src/ctapipe_io_nectarcam/tests/test_nectarcameventsource.py::test_loop_over_events. Could you please briefly explain it to me ? Thanks !

vmarandon commented 1 year ago

This is because the even file contain the even numbers. First event in this file is 2.

I would suggest to change the :

FIRST_EVENT_NUMBER_IN_FILE = 1 by : FIRST_EVENT_NUMBER_IN_FILE = 2

and : assert event.index.event_id == FIRST_EVENT_NUMBER_IN_FILE + i by : assert event.index.event_id == FIRST_EVENT_NUMBER_IN_FILE + i*2

jlenain commented 1 year ago

Ah OK, thanks @vmarandon . And if we were to use an input file with odd events instead, would that break again the test? Edit: ok, yes it would... So we should be careful to match this variable with the kind of input test file, not very clean but maybe let's stick to it at the moment?

vmarandon commented 1 year ago

Perhaps just put a comment in the test to explain what needs to be done if file change ?