adding if condition to remove keys

[vuillaut]

DL1 to DL2 fails on merged DL1 files where images have been removed.

Fixed this and added unit tests for merged DL1 files.

[rlopezcoto]

damn, that one was missed... it means that for this production, we either have to merge at a dl2 level or release a bugfix version for the processing... @morcuended were you thinking on performing the merge before producing the DL2?

[maxnoe]

I would suggest a bug fix release, since no other PRs have been merged yet.

[maxnoe]

Apperently the next version of sklearn has been released with the removal of joblib. You just need to replace the imports of joblib with import joblib and add it to the dependencies.

[vuillaut]

Apperently the next version of sklearn has been released with the removal of joblib. You just need to replace the imports of joblib with import joblib and add it to the dependencies.

shall we fix the version of joblib to ensure compatibility of the models?

[maxnoe]

@vuillaut I don't think joblib affects compatibility. I tested this for the aict tools and there were no problems with loading models stored with older versions of joblib with newer versions of joblib.

The story is different for sklearn, so you should always load a model with the same version of sklearn that was used to train it.

[rlopezcoto]

Apperently the next version of sklearn has been released with the removal of joblib. You just need to replace the imports of joblib with import joblib and add it to the dependencies.

The joblib issue is addressed here:
https://github.com/cta-observatory/cta-lstchain/pull/338/files
as soon as this is addressed, it can be merged.

[maxnoe]

As the other PR also does other changes, I would rather merge this here fast, that be simpler, right?

[rlopezcoto]

As the other PR also does other changes, I would rather merge this here fast, that be simpler, right?

but without doing any joblib modification, right? Then I agree (as soon as it passes travis checks)

[maxnoe]

@vuillaut with pickle protocol 5 (python 3.8) and numpy 1.18, joblib becomes obsolete and we can just use the stdlib pickle module

[maxnoe]

but without doing any joblib modification, right? Then I agree (as soon as it passes travis checks)

Why? no need to revert this. The changes are even exactly the same, so it won't even create a merge conflict.

[morcuended]

damn, that one was missed... it means that for this production, we either have to merge at a dl2 level or release a bugfix version for the processing... @morcuended were you thinking on performing the merge before producing the DL2?

No, we also produce DL2 files subrun-wise

[morcuended]

Is the problem with missing joblib module fixed in this PR?

[rlopezcoto]

Why? no need to revert this. The changes are even exactly the same, so it won't even create a merge conflict.

I did not see that it was already included. So fine then

[vuillaut]

• Concerning joblib, I made the changes already don't worry
• Concerning merge, it's a problem for the IRF production at least (and needs to be fixed anyway)

[rlopezcoto]

Is the problem with missing joblib module fixed in this PR?

should be

No, we also produce DL2 files subrun-wise

it seems that @vuillaut needs this, so I'll do the v0.5.1 release and install it in the IT cluster

[vuillaut]

@vuillaut with pickle protocol 5 (python 3.8) and numpy 1.18, joblib becomes obsolete and we can just use the stdlib pickle module

good to know.
To keep in mind when we support only python >= 3.8 ;-)

[codecov]

Codecov Report

Merging #409 into master will increase coverage by 0.06%.
The diff coverage is 72.22%.

@@            Coverage Diff             @@
##           master     #409      +/-   ##
==========================================
+ Coverage   41.50%   41.57%   +0.06%     
==========================================
  Files          76       76              
  Lines        6141     6153      +12     
==========================================
+ Hits         2549     2558       +9     
- Misses       3592     3595       +3     

Impacted Files	Coverage Δ
lstchain/scripts/lstchain_dl1_to_dl2.py	0.00% <0.00%> (ø)
lstchain/reco/dl1_to_dl2.py	65.77% <100.00%> (ø)
lstchain/scripts/tests/test_lstchain_scripts.py	98.64% <100.00%> (+0.21%)	⬆️
lstchain/tests/test_lstchain.py	100.00% <100.00%> (ø)
lstchain/version.py	43.93% <0.00%> (-1.52%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5cc9864...fb5c1cf. Read the comment docs.

[vuillaut]

Ready for review.

[vuillaut]

And we are celebrating 100th unit test 🥳