fix: prevent crashes on zero-length dimensions in echodata conversion - #1624
Conversation
Partial/truncated raw files (e.g. from autonomous vehicle deployments) can produce channels with zero pings, causing: - Division by zero in set_zarr_encodings() chunk calculation - ValueError in _nan_timestamp_handler() on empty ping_time arrays - Encoding failure on all-NaN time variables Changes: - utils/coding.py: Skip all-NaN time vars in set_time_encodings(); handle zero-length and scalar dims in set_zarr_encodings() - convert/set_groups_base.py: Return [np.nan] cleanly when ping_time arrays are empty in _nan_timestamp_handler()
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1624 +/- ##
==========================================
+ Coverage 84.90% 85.53% +0.62%
==========================================
Files 79 79
Lines 6998 7010 +12
==========================================
+ Hits 5942 5996 +54
+ Misses 1056 1014 -42
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @beatfactor, sorry for the delay... I can review this PR and #1635 this week, if that works for you? If so, could you also take a look at the pre-commit failures and resolve the merge conflicts? I'd be happy to help, but I can't push to those PRs... cheers! |
|
Sure, I'll take a look over the next few days. |
|
Hi @beatfactor, i'm adding this for the next release, due at the end of the month! cheers! |
…ro-length-dims Resolved conflict in echopype/convert/set_groups_base.py: - kept the upstream SONAR_MODELS/model_family lookup, and - preserved the empty-ping-times guard so partial/truncated raw files still return [np.nan] instead of crashing in np.array(...).min().
LOCEANlloydizard
left a comment
LOCEANlloydizard
left a comment
There was a problem hiding this comment.
Hey @beatfactor looks good to me! Could we/should we just add a few regression tests covering the zero-length/all-NaN cases fixed here? I would do it myself, but I can’t push directly to the PR.. (maybe we could enable the “Allow edits from maintainers” option if not?) cheers!
…ck-org#1624) - test_coding.py: cover set_time_encodings skipping all-NaN time vars, and set_zarr_encodings on zero-length dims + scalar variables. - test_set_groups_common.py: cover _nan_timestamp_handler for empty ping_time dicts, all-empty channel arrays, populated ping_time, and passthrough.
LOCEANlloydizard
left a comment
LOCEANlloydizard
left a comment
There was a problem hiding this comment.
thx @beatfactor for this and for adding the tests too! i'll merge this now, thx!
3 participants
Partial/truncated raw files can produce channels with zero pings, causing:
Changes: