Filter events not droping nan on all columns anymore #526
Conversation
Ouch, other than the failing CI test of course! |
Seems that regression_features is simply missing in one of the unit tests |
lstchain/reco/utils.py
Outdated
|
|
||
| if dropna: | ||
| _finite_params = list(set(finite_params + events.columns)) |
There was a problem hiding this comment.
I think you got a logic error here. You still compute events.isnull on all colums. and there is no any(axis=1) her.
Also _finit_params now contains all columns in events, right? I think the correct solutino would just be:
nans = events[finite_params].isnull()
filter &= ~nans.any(axis=1) # not any, so all have to be finite
There was a problem hiding this comment.
damn, I tried to be too quick, thanks.
Codecov Report
@@ Coverage Diff @@
## master #526 +/- ##
==========================================
+ Coverage 41.66% 41.82% +0.15%
==========================================
Files 77 77
Lines 6518 6534 +16
==========================================
+ Hits 2716 2733 +17
+ Misses 3802 3801 -1
Continue to review full report at Codecov.
|
|
Good! Would be great to merge this so that @morcuended can test the full chain @ La Palma |
@morcuended I wanted to make an update on your PR #525 , made one in your branch, then finally rebased from yours considering @moralejo comments, hence the history of commits here.
Finally, your first idea was in the right direction.