Add OptaSense ODH4 format reader - #847
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughChangesODH4 HDF5 FiberIO support now includes metadata parsing, coordinate construction, format detection, scanning, reading, registration, sample data, documentation, and validation tests. ODH4 Fiber I/O 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/odh4/utils.py`:
- Around line 72-81: Update the sampling-rate validation near rate conversion to
reject all non-finite values, including NaN and positive or negative infinity,
while preserving rejection of non-positive rates; raise InvalidFiberFileError
before deriving time steps. Add regression coverage for NaN and inf inputs.
🪄 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: bb0da5fd-dbea-4575-a196-e3f69fd8d69c
📒 Files selected for processing (8)
dascore/data_registry.txtdascore/io/odh4/__init__.pydascore/io/odh4/core.pydascore/io/odh4/utils.pydocs/changelog.qmdpyproject.tomltests/test_io/test_common_io.pytests/test_io/test_odh4/test_odh4.py
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9db1f2fe0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| step_from_rate = dc.to_timedelta64(1 / rate) | ||
| channel_range = int(attrs["channel_end"]) - int(attrs["channel_start"]) | ||
| time_consistent = abs(step_from_span - step_from_rate) <= 0.01 * step_from_rate | ||
| channel_consistent = abs(channel_range - n_channel) <= 1 |
There was a problem hiding this comment.
When channel_end - channel_start == n_channel + 1, this symmetric tolerance accepts the metadata even though the documented exclusive and inclusive conventions only permit n_channel and n_channel - 1, respectively. The generated channel coordinate then silently ignores the extra declared channels, allowing a corrupt file to be read with misleading coordinates; restrict the accepted range to those two valid values.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done — the tolerance now accepts exactly the exclusive (n) and inclusive (n-1) conventions; a span of n+1 raises InvalidFiberFileError, with a regression test.
| time_end = dc.to_datetime64(attrs["endtime"]) | ||
| # Deriving the step from the time span avoids accumulating the | ||
| # nanosecond truncation of 1/rate over long files. | ||
| step_from_span = (time_end - time_start) / n_time |
There was a problem hiding this comment.
For an inclusive-bound file with at least about 100 samples, (endtime - starttime) == (n_time - 1) / rate passes the 1% consistency tolerance, but dividing that span by n_time produces a step smaller than 1 / rate. The generated last timestamp consequently falls roughly one sampling interval before the declared inclusive endtime; distinguish inclusive from exclusive metadata and use the corresponding n_time - 1 or n_time denominator.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done — the step denominator is now chosen by whichever convention the span is closer to, so inclusive-bound files keep the exact 1/rate step; regression test asserts the step survives an inclusive endtime.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #847 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 170 173 +3
Lines 18374 18469 +95
=========================================
+ Hits 18374 18469 +95
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: |
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/odh4/utils.py`:
- Around line 83-97: Update the consistency checks in the ODH4 validation flow
to enforce exclusive bounds: derive step_from_span using n_time only, without
selecting n_time - 1, and require channel_range to equal n_channel exactly
rather than accepting n_channel - 1. Preserve the existing InvalidFiberFileError
path for inconsistent files.
🪄 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: e5bc98e3-8ef2-493b-92cf-aef2cbb46b95
📒 Files selected for processing (2)
dascore/io/odh4/utils.pytests/test_io/test_odh4/test_odh4.py
Adds a FiberIO reader for the OptaSense ODH4 HDF5 layout (raw_data dataset with self-describing root attrs), used e.g. by the UW-Madison SURF deployment in the PubDAS Global DAS Month dataset. Detection requires the complete attr set since the names are informal; the reader warns when a file's endtime/channel_end disagree with its data shape.
86cde05 to
67f88a3
Compare
1 participant
Description
Third PR of the format-reader series (#844, #846). Adds a FiberIO reader for the OptaSense ODH4 HDF5 layout: a
raw_datadataset of shape (channel, time) with self-describing root attrs (start/end times, sampling rate, channel range/spacing, gauge length, units, scale factor to strain). The registeredoptasense_odh4_1.h5test file is a trimmed excerpt of the UW-Madison SURF deployment from the PubDAS Global DAS Month (Feb 2023) dataset.Design notes:
GL m,channel spacing m, ...) so partial matches must not claim unrelated files; a near-miss negative test pins this.endtime - starttime == n/rate,channel_end - channel_start == n_channels). The reader derives the time step from the span (avoids ns-truncation drift on long files) and raisesInvalidFiberFileErrorwhen attrs disagree with the data shape beyond inclusive-writer slack — a transposed or corrupt file must not read with plausible-but-wrong coordinates (tested with a transposed copy).channelcoord on the distance dim, so they stay correct under trimming (channel-range attrs would go stale). Known interaction: lazy-spool content summaries narrow associated coords only on load — a general property of associated coords, noted for a possible core follow-up.raw_data_units("phase shift in radians") maps toradians; unparseable descriptions yield unset units rather than a pint error that would abort a directory scan.Changelog
ODH4format for OptaSense ODH4 HDF5 files, e.g. the UW-Madison SURF deployment in PubDAS Global DAS Month.Checklist
I have (if applicable):
Summary by CodeRabbit
New Features
Bug Fixes
Documentation