search

cta-observatory / cta-lstchain

LST prototype testbench chain
https://cta-observatory.github.io/cta-lstchain/
BSD 3-Clause "New" or "Revised" License
22 stars 77 forks source link

Correct the search of interleaved files in the official tree #1192

Closed FrancaCassol closed 3 months ago

FrancaCassol commented 7 months ago

The present official tree path for DL1 files contains also the "short" code version: E.g. /fefs/aswg/data/real/DL1/[date]/v0.10

This was not considered in the interleaved search function, this PR correct the search.

codecov[bot] commented 7 months ago

Codecov Report

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

Comparison is base (107f8ce) 74.22% compared to head (404c8a2) 74.23%. Report is 2 commits behind head on main.

Files Patch % Lines
...pts/onsite/onsite_create_cat_B_calibration_file.py 14.28% 6 Missing :warning:
...nsite_create_cat_B_calibration_files_with_batch.py 14.28% 6 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1192 +/- ## ======================================= Coverage 74.22% 74.23% ======================================= Files 130 130 Lines 13279 13273 -6 ======================================= - Hits 9857 9853 -4 + Misses 3422 3420 -2 ```

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

FrancaCassol commented 5 months ago

Hi @morcuended, the tests of the code were added in other PRs, which type of tests do you mean? I can think to them for an other PR, this PR is just a bug correction. @maxnoe could we merge this PR?

maxnoe commented 5 months ago

I can think to them for an other PR, this PR is just a bug correction.

The tests were passing before, the tests were unchanged and are still passing. Yet you say there was a bug. So the tests didn't cover the bug and we should add a new test that tests that it has been fixed correctly.

I.e. that test should fail before your code changes and pass after.

morcuended commented 5 months ago

Hi @morcuended, the tests of the code were added in other PRs, which type of tests do you mean? I can think to them for an other PR, this PR is just a bug correction.

Ah, ok, I see that the calib B tool is 86% covered. The onsite calib B script is 30% covered.

The change in this PR is not really a bugfix. It depends on the directory tree in the container. You could assume the same directory tree in the tests as well