Add Silixa H5 Carina (netCDF-shell) variant reader - #849
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdded Silixa H5 version 2 support for Carina netCDF-shell files. The reader handles Carina metadata, ChangesSilixa H5 Carina support
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dascore/io/silixah5/utils.py`:
- Around line 183-186: Update the ChannelMap validation in the relevant utility
flow before indexing resource[_CARINA_CHANNEL_MAP][()] to require a dataset-like
node, raising InvalidFiberFileError for groups or other non-dataset objects;
retain the one-dimensional validation for datasets and add a regression test
covering a ChannelMap group.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5de518ae-5bd2-4582-a670-3a3a196d7ba6
📒 Files selected for processing (8)
dascore/data_registry.txtdascore/io/silixah5/__init__.pydascore/io/silixah5/core.pydascore/io/silixah5/utils.pydocs/changelog.qmdpyproject.tomltests/test_io/test_common_io.pytests/test_io/test_silixah5/test_silixa_carina.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #849 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 164 164
Lines 17950 18020 +70
=========================================
+ Hits 17950 18020 +70
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
✅ Documentation built: |
Adds version 2 of the Silixa_H5 format for Carina deployments that write HDF5 through a netCDF library (e.g. INGV Mt Etna in the PubDAS Global DAS Month dataset): the Silixa attrs sit on the file root, data live in a Fiber int16 counts dataset, and a ChannelMap dataset places each stored column on the physical fiber. The file's netCDF coordinate variables are empty or zeroed, so time derives from the StartTime microsecond epoch and Samplerate attrs and distance from Start Distance and SpatialResolution (times the fiber length multiplier, which reproduces the file's Stop Distance exactly) mapped through ChannelMap. NETCDF_CF keeps rejecting these files; a cross-check test pins that.
402d1f5 to
7388929
Compare
The Carina reader (#849) now claims this file but merged without removing the skip entry, so its common IO tests were silently skipped.
The Carina reader (#849) now claims this file but merged without removing the skip entry, so its common IO tests were silently skipped.
The Carina reader (#849) now claims this file but merged without removing the skip entry, so its common IO tests were silently skipped.
1 participant
Description
Final PR of the format-reader series (#844, #846, #847, #848). Adds
SilixaH5V2, supporting Silixa Carina deployments that write HDF5 through a netCDF library — e.g. the INGV Mt Etna array from PubDAS Global DAS Month, whose trimmed sample this series registered assilixa_h5_ingv_1.h5(in #844).These files carry the same Silixa attr family the existing
Silixa_H5reader keys on, but on the file root instead of anAcousticdataset; samples live in aFiberint16 counts dataset (time × channel), and aChannelMapdataset places each stored column on the physical fiber (channels 143–413 in the sample). The file's own netCDF coordinate variables are empty or zeroed, so:StartTimeµs-epoch andSamplerateroot attrs (notSamplingFrequency[Hz], which is the pre-decimation rate).Start Distance (m)+ physical_channel ×SpatialResolution[m]×Fibre Length Multiplier, mapped throughChannelMap— this reproduces the file'sStop Distanceexactly. Non-contiguous (but bijective) maps produce per-channel distance values; a non-bijective, wrongly-shaped, or empty map raisesInvalidFiberFileErrorrather than guessing.Implementation notes:
SilixaH5V1was refactored to hook-based dispatch (_data_name/_version_check/_attr_getter/_patch_getter) so V2 shares the method bodies instead of duplicating them; V1 behavior is unchanged (existing test file still passes the common suite).NETCDF_CFkeeps rejecting these files (they have_NCPropertiesbut noConventionsattr), complementing the guard test from Add guard test: NETCDF_CF rejects Silixa Carina netCDF-flavored HDF5 #844. This PR adds its own registry entry forsilixa_h5_ingv_1.h5(it is based on dev before Add guard test: NETCDF_CF rejects Silixa Carina netCDF-flavored HDF5 #844). Merge order note: once Add guard test: NETCDF_CF rejects Silixa Carina netCDF-flavored HDF5 #844 merges, rebasing this PR must drop the duplicate registry line and also remove Add guard test: NETCDF_CF rejects Silixa Carina netCDF-flavored HDF5 #844'sSKIP_DATA_FILESentry for the file, since a reader now claims it — I will handle the rebase.Silixa_H5(matching how Add Aragon Photonics HDAS format reader (two variants) #848 numbers the HDAS sibling variants). If you'd rather reserve numeric versions for genuine vendor revisions, this could be renamed before merge — version strings do get baked into indexes.Changelog
SILIXA_H5format for the Carina netCDF-shell variant, with Silixa attrs on the file root and theFibercounts dataset placed viaChannelMap.Checklist
I have (if applicable):
Summary by CodeRabbit
New Features
Bug Fixes
Documentation