-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fill in NaN for missing EK80 coefficients [all tests ci] #1458
Fill in NaN for missing EK80 coefficients [all tests ci] #1458
Conversation
for more information, see https://pre-commit.ci
The failed tests do not appear to be related to the changes made in this PR. I'll check soon. |
Yeah, I tried one of the failed tests with an older scipy version and it passed, but after upgrading to the latest scipy that test failed. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1458 +/- ##
==========================================
+ Coverage 83.52% 85.33% +1.81%
==========================================
Files 64 72 +8
Lines 5686 6579 +893
==========================================
+ Hits 4749 5614 +865
- Misses 937 965 +28
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I pushed something small to cast numpy float arrays to integers when indexing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leewujung I think what you have looks good. Let me know what you think of these small changes I made.
@ctuguinay : Thanks for the changes and the added test! Great that you caught this indexing problem. I had forgotten that having The only thing I have is github warns that there's no |
This PR change EK80
set_vendor
to fill innp.nan
for missing filter coeffs across all channels.Currently we have below in our test file:
D20210330-T123857.raw
that does not contain any filter coeffsthree_ensemble-Phase0-D20240506-T053349-0.raw
that only contains filter coeffs from 1 of the 2 channels, which seems to be due to ping sequencing. This file is from Error storing filter coefficients for sequenced ek80 file #1317.This addresses #1317 and #1413.