Expose exact coordinates through scan payloads - #754
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesThe scan API adds Scan payload and coordinate flow
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #754 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 162 164 +2
Lines 16512 17191 +679
==========================================
+ Hits 16512 17191 +679
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:
|
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dascore/io/core.py (1)
748-779: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHonor
snapin the default read-backed scan.Line 775 calls
self.read(resource)with its defaults, so formatters relying on this fallback still return snapped coordinates forscan_payloads(..., snap=False)when their reader supportssnaporsnap_dims.Proposed fix
def scan(self, resource, snap: bool = True, **kwargs) -> list[ScanPayload]: + read_params = inspect.signature(self.read).parameters + read_kwargs = {} + if "snap" in read_params: + read_kwargs["snap"] = snap + elif "snap_dims" in read_params: + read_kwargs["snap_dims"] = snap try: - spool = self.read(resource) + spool = self.read(resource, **read_kwargs)Add a read-only formatter regression test with an explicit exact-read parameter.
🤖 Prompt for 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. In `@dascore/io/core.py` around lines 748 - 779, Update the default scan implementation in FiberIO.scan to propagate the caller’s snap setting to the read-backed fallback, including the formatter’s supported exact-coordinate read parameter such as snap or snap_dims. Preserve the existing NotImplementedError handling and payload conversion, and add a regression test for a read-only formatter verifying scan(..., snap=False) uses exact-read behavior.
🤖 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.
Outside diff comments:
In `@dascore/io/core.py`:
- Around line 748-779: Update the default scan implementation in FiberIO.scan to
propagate the caller’s snap setting to the read-backed fallback, including the
formatter’s supported exact-coordinate read parameter such as snap or snap_dims.
Preserve the existing NotImplementedError handling and payload conversion, and
add a regression test for a read-only formatter verifying scan(..., snap=False)
uses exact-read behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b3b43ef8-89de-427b-95e7-ea87a691c787
📒 Files selected for processing (25)
dascore/__init__.pydascore/io/__init__.pydascore/io/core.pydascore/io/dasdae/core.pydascore/io/dasdae/utils.pydascore/io/dashdf5/core.pydascore/io/dashdf5/utils.pydascore/io/febus/core.pydascore/io/febus/g1utils.pydascore/io/febus/t1utils.pydascore/io/gdr/utils_das.pydascore/io/h5simple/utils.pydascore/io/netcdf/core.pydascore/io/neubrex/utils_rfs.pydascore/io/optodas/core.pydascore/io/optodas/utils.pydascore/io/prodml/core.pydascore/io/prodml/utils.pydascore/io/terra15/core.pydascore/io/terra15/utils.pydocs/changelog.qmddocs/contributing/new_format.qmdtests/test_io/test_common_io.pytests/test_io/test_io_core.pytests/test_io/test_terra15/test_terra15.py
|
✅ Documentation built: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
dascore/io/optodas/utils.py (1)
51-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider passing
unitsto the distance coordinate builders.In the loop above,
unitis extracted from the header (line 37) and is correctly passed toget_coordfor the"time"dimension (line 47). However,unitis not currently passed when building the"distance"dimension, which could cause the coordinate to lack unit metadata. Consider passing it to bothget_coordandget_exact_coordto ensure the metadata is retained.💡 Proposed optional refactor
- if snap: - coord = get_coord(data=distance) - else: - coord = get_exact_coord(distance) + if snap: + coord = get_coord(data=distance, units=unit) + else: + coord = get_exact_coord(distance, units=unit)🤖 Prompt for 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. In `@dascore/io/optodas/utils.py` around lines 51 - 54, Update the distance-dimension branch to pass the extracted unit to both get_coord and get_exact_coord, matching the time-dimension handling and preserving unit metadata on the resulting coordinate.
🤖 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.
Nitpick comments:
In `@dascore/io/optodas/utils.py`:
- Around line 51-54: Update the distance-dimension branch to pass the extracted
unit to both get_coord and get_exact_coord, matching the time-dimension handling
and preserving unit metadata on the resulting coordinate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bfdb57da-d9a3-42db-8b8b-571873afeaf8
📒 Files selected for processing (15)
dascore/io/core.pydascore/io/dasdae/utils.pydascore/io/dashdf5/utils.pydascore/io/febus/g1utils.pydascore/io/febus/t1utils.pydascore/io/gdr/utils_das.pydascore/io/h5simple/utils.pydascore/io/netcdf/core.pydascore/io/neubrex/utils_rfs.pydascore/io/optodas/utils.pydascore/io/prodml/utils.pydascore/io/terra15/utils.pydascore/io/utils.pytests/test_io/test_io_core.pytests/test_io/test_netcdf/test_netcdf.py
🚧 Files skipped from review as they are similar to previous changes (1)
- dascore/io/core.py
39c7354 to
06548e6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/core.py`:
- Around line 149-185: Extend _validate_scan_payload to validate every
ScanPayload field: require valid dimension names, shape, dtype, and attrs
types/values rather than only checking required keys. In strict mode, require
coords to be a full CoordManager and ensure dims exactly equals coords.dims and
shape exactly equals coords.shape; make scan_payloads use this strict validation
before returning payloads.
- Around line 839-846: Initialize read_kwargs from the caller-provided kwargs
before applying the snap or snap_dims translation in the read delegation logic.
Preserve all existing scan filters and reader options while allowing the
translated snap argument to override or augment the corresponding key.
🪄 Autofix (Beta)
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
Run ID: b62aa2a5-eadd-4f17-89ad-74b6a511ea2b
📒 Files selected for processing (39)
dascore/__init__.pydascore/io/__init__.pydascore/io/core.pydascore/io/dasdae/core.pydascore/io/dasdae/utils.pydascore/io/dashdf5/core.pydascore/io/dashdf5/utils.pydascore/io/febus/core.pydascore/io/febus/g1utils.pydascore/io/febus/t1utils.pydascore/io/gdr/utils_das.pydascore/io/h5simple/utils.pydascore/io/netcdf/core.pydascore/io/neubrex/utils_rfs.pydascore/io/optodas/core.pydascore/io/optodas/utils.pydascore/io/prodml/core.pydascore/io/prodml/utils.pydascore/io/terra15/core.pydascore/io/terra15/utils.pydascore/io/utils.pydocs/changelog.qmddocs/contributing/documentation.qmddocs/contributing/new_format.qmddocs/notes/coordinate_internals.qmddocs/notes/dft_notes.qmddocs/notes/doc_strategy.qmddocs/notes/notes.qmddocs/notes/patch_attrs.qmddocs/tutorial/coords.qmddocs/tutorial/file_io.qmddocs/tutorial/remote_patches.qmddocs/tutorial/spool.qmdscripts/_templates/_quarto.ymltests/test_io/test_common_io.pytests/test_io/test_io_core.pytests/test_io/test_netcdf/test_netcdf.pytests/test_io/test_optodas/test_optodas.pytests/test_io/test_terra15/test_terra15.py
🚧 Files skipped from review as they are similar to previous changes (32)
- dascore/io/neubrex/utils_rfs.py
- dascore/io/dashdf5/core.py
- dascore/io/utils.py
- tests/test_io/test_optodas/test_optodas.py
- tests/test_io/test_netcdf/test_netcdf.py
- dascore/io/optodas/core.py
- dascore/io/dasdae/core.py
- dascore/io/prodml/core.py
- dascore/io/gdr/utils_das.py
- docs/contributing/documentation.qmd
- dascore/init.py
- docs/changelog.qmd
- dascore/io/terra15/core.py
- docs/contributing/new_format.qmd
- docs/tutorial/file_io.qmd
- docs/notes/patch_attrs.qmd
- dascore/io/febus/core.py
- docs/tutorial/coords.qmd
- tests/test_io/test_terra15/test_terra15.py
- docs/notes/notes.qmd
- dascore/io/init.py
- dascore/io/optodas/utils.py
- dascore/io/terra15/utils.py
- tests/test_io/test_common_io.py
- docs/notes/doc_strategy.qmd
- dascore/io/dasdae/utils.py
- dascore/io/prodml/utils.py
- dascore/io/febus/g1utils.py
- docs/tutorial/remote_patches.qmd
- docs/notes/dft_notes.qmd
- docs/notes/coordinate_internals.qmd
- tests/test_io/test_io_core.py
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dascore/io/core.py (1)
1568-1571: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winOnly suppress
UnknownFiberFormatErrorhere. CatchingExceptionhides realget_format()failures and can misclassify a directory as non-unit, sending its members through traversal instead of surfacing the error.🤖 Prompt for 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. In `@dascore/io/core.py` around lines 1568 - 1571, In the format-checking logic around get_format, replace the broad Exception handler with a catch for UnknownFiberFormatError only, returning False for that expected unsupported-format case. Allow all other get_format failures, including directory-related errors, to propagate instead of being classified as non-unit.Source: Linters/SAST tools
🤖 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/febus/t1utils.py`:
- Line 69: Rename the format-related parameters in _scan_t1 to avoid shadowing
the built-in format(), and update all callers accordingly, including
FebusT1V1.scan() in the file-format keyword path. If _scan_t1 does not use these
parameters, remove them from its signature and call sites instead.
---
Outside diff comments:
In `@dascore/io/core.py`:
- Around line 1568-1571: In the format-checking logic around get_format, replace
the broad Exception handler with a catch for UnknownFiberFormatError only,
returning False for that expected unsupported-format case. Allow all other
get_format failures, including directory-related errors, to propagate instead of
being classified as non-unit.
🪄 Autofix (Beta)
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
Run ID: 0db95b5d-7287-4121-b6f2-ee6efc21a2ad
📒 Files selected for processing (39)
dascore/__init__.pydascore/io/__init__.pydascore/io/core.pydascore/io/dasdae/core.pydascore/io/dasdae/utils.pydascore/io/dashdf5/core.pydascore/io/dashdf5/utils.pydascore/io/febus/core.pydascore/io/febus/g1utils.pydascore/io/febus/t1utils.pydascore/io/gdr/utils_das.pydascore/io/h5simple/utils.pydascore/io/netcdf/core.pydascore/io/neubrex/utils_rfs.pydascore/io/optodas/core.pydascore/io/optodas/utils.pydascore/io/prodml/core.pydascore/io/prodml/utils.pydascore/io/terra15/core.pydascore/io/terra15/utils.pydascore/io/utils.pydocs/changelog.qmddocs/contributing/documentation.qmddocs/contributing/new_format.qmddocs/notes/coordinate_internals.qmddocs/notes/dft_notes.qmddocs/notes/doc_strategy.qmddocs/notes/notes.qmddocs/notes/patch_attrs.qmddocs/tutorial/coords.qmddocs/tutorial/file_io.qmddocs/tutorial/remote_patches.qmddocs/tutorial/spool.qmdscripts/_templates/_quarto.ymltests/test_io/test_common_io.pytests/test_io/test_io_core.pytests/test_io/test_netcdf/test_netcdf.pytests/test_io/test_optodas/test_optodas.pytests/test_io/test_terra15/test_terra15.py
🚧 Files skipped from review as they are similar to previous changes (36)
- docs/tutorial/spool.qmd
- dascore/io/dasdae/core.py
- dascore/init.py
- dascore/io/utils.py
- docs/notes/notes.qmd
- docs/tutorial/file_io.qmd
- docs/contributing/documentation.qmd
- docs/tutorial/coords.qmd
- dascore/io/optodas/core.py
- docs/notes/patch_attrs.qmd
- scripts/_templates/_quarto.yml
- tests/test_io/test_netcdf/test_netcdf.py
- dascore/io/gdr/utils_das.py
- docs/contributing/new_format.qmd
- docs/notes/dft_notes.qmd
- dascore/io/dashdf5/core.py
- docs/tutorial/remote_patches.qmd
- dascore/io/init.py
- dascore/io/h5simple/utils.py
- dascore/io/prodml/core.py
- dascore/io/terra15/core.py
- dascore/io/netcdf/core.py
- tests/test_io/test_optodas/test_optodas.py
- dascore/io/optodas/utils.py
- dascore/io/prodml/utils.py
- dascore/io/terra15/utils.py
- tests/test_io/test_common_io.py
- dascore/io/dasdae/utils.py
- dascore/io/dashdf5/utils.py
- docs/changelog.qmd
- dascore/io/febus/core.py
- dascore/io/neubrex/utils_rfs.py
- tests/test_io/test_terra15/test_terra15.py
- dascore/io/febus/g1utils.py
- docs/notes/coordinate_internals.qmd
- tests/test_io/test_io_core.py
92a189a to
0d8b9cf
Compare
d-chambers
left a comment
d-chambers
left a comment
There was a problem hiding this comment.
Nice shape overall — splitting _iter_scan_results out so scan and scan_payloads share one traversal is the right factoring, the validation helper gives a real contract boundary, and the format audit table in the description is genuinely useful. The docs work (coordinate_internals, new_format snap section) is a clear improvement.
My main concern is get_exact_coord. CoordSegmented.from_array(..., tolerance=0) degenerates badly on jittery stored timestamps — which is exactly what GPS-stamped DAS time arrays look like — and because terra15._get_raw_time_coord is on the read path, this lands as a performance and representation regression on an existing public API, not just on the new one.
Measured on this branch vs. dev, using the checked-in trimmed terra15 test file (3,620 samples):
| dev | this branch | |
|---|---|---|
dc.read(path, snap_dims=False) |
0.154 s | 0.812 s |
| resulting time coord | CoordMonotonicArray |
CoordSegmented, 1001 segments |
And synthetic monotonic arrays with ±3 ns jitter on a 1 µs step:
| n | get_exact_coord |
get_coord(data=...) |
segments |
|---|---|---|---|
| 1,000 | 0.125 s | 0.0003 s | 122 |
| 10,000 | 1.21 s | 0.0007 s | 1,139 |
| 50,000 | 6.06 s | 0.0018 s | 5,512 |
| 100,000 | 11.6 s | — | ~11,000 |
Roughly 120 µs per segment, so a 1M-sample file extrapolates to ~2 minutes. Both problems come from the same place: from_array's run detection (a difference joins a run only when it equals a neighbor) means jitter turns nearly every sample into its own seam, so you get an n/2-segment object that is slower to build, larger in memory, and less useful than the CoordMonotonicArray it replaced.
The clean cases are fine — uniform-with-one-gap gives 2 segments as intended — so the fix is a guard rather than a redesign: bail out to get_coord(data=...) when the detected segment count exceeds some small fraction of len(values). That preserves the real win (exact values plus queryable seams for genuinely piecewise-uniform data) without the pathological path. Ideally detect this before materializing all the segments so the cost is bounded too. Worth a test with a jittery array asserting the coord type and that values round-trip exactly.
Three smaller items inline. Everything else — the validation helper, provenance ownership, forwarding tests, conformance test — reads well to me.
| def get_exact_coord(values, units=None) -> BaseCoord: | ||
| """Return an exact coordinate, including for non-monotonic values.""" | ||
| try: | ||
| return CoordSegmented.from_array(values, tolerance=0, units=units) |
There was a problem hiding this comment.
This is where the segment explosion originates (see review body for numbers). With tolerance=0, from_array never raises on a jittery-but-monotonic array — it happily returns a CoordSegmented with ~n/2 segments — so the CoordError fallback below only catches non-monotonic/duplicate/multidim input and never the case that actually needs it.
Suggest an explicit degeneracy guard, e.g.:
def get_exact_coord(values, units=None) -> BaseCoord:
"""Return an exact coordinate, including for non-monotonic values."""
try:
out = CoordSegmented.from_array(values, tolerance=0, units=units)
except CoordError:
return get_coord(data=values, units=units)
# Jittery arrays segment into ~n/2 runs; a plain monotonic array is
# smaller, faster to build, and equally exact.
if len(getattr(out, "segments", ())) > _MAX_SEGMENT_FRACTION * len(values):
return get_coord(data=values, units=units)
return outget_coord(data=...) is exact for these inputs — its tolerant inference only collapses to CoordRange when the array really is near-uniform, which is the case where segmentation would have produced one segment anyway. A pre-check inside from_array that avoids building all the segments first would be better still, since the cost is ~120 µs/segment.
There was a problem hiding this comment.
Addressed in 0dca954. Added a _is_over_segmented pre-check to get_exact_coord: for arrays large enough for the cost to matter, it cheaply predicts the segment count (mirroring from_arrays run detection) and falls back to get_coord(data=...) when segments would exceed one per ten samples. I used a pre-check rather than the post-hoc len(out.segments) guard so the degenerate path never materializes the segments at all (the cost is ~120 µs/segment). Genuinely piecewise-uniform arrays still segment and keep their queryable seams. Regression tests (jittery array stays monotonic + round-trips exactly; piecewise-uniform stays segmented) added in aea075c.
| time = _get_time_node(data_node)[:] | ||
| return get_coord(data=to_datetime64(time)) | ||
| values = to_datetime64(time) | ||
| return get_exact_coord(values, units="s") |
There was a problem hiding this comment.
Two behavior changes on the read path, not just scan — _get_raw_time_coord is what _read_terra15(..., snap_dims=False) calls:
get_exact_coordhere is what makesdc.read(terra15_path, snap_dims=False)5x slower and return a 1001-segmentCoordSegmentedinstead of aCoordMonotonicArray(numbers in the review body). Fixingget_exact_coordresolves this.units="s"is newly attached to adatetime64coord that previously had none. It matches thesnap_dims=Truebranch above so it's probably the intended normalization, but it changes existingdc.readoutput and isn't in the changelog. Worth a line there either way.
There was a problem hiding this comment.
Addressed in 0dca954. The get_exact_coord guard resolves the read-path regression: dc.read(terra15, snap_dims=False) is back to ~0.12 s and returns a CoordMonotonicArray again instead of a 1001-segment CoordSegmented. The units="s" normalization is intentional (matches the snap_dims=True branch) and is now called out in the changelog.
| try: | ||
| get_format(path) | ||
| except Exception: | ||
| except UnknownFiberFormatError: |
There was a problem hiding this comment.
This narrowing looks unrelated to the scan-payload work, and it makes directory indexing more fragile. is_directory_format is called from indexer.py for every directory encountered during traversal, and get_format dispatches to arbitrary FiberIO.get_format implementations — an unreadable directory (PermissionError) or a formatter that raises something unexpected on a weird path will now abort the whole index instead of the directory simply not being a scan unit.
The blanket except Exception was admittedly too broad, but I'd rather see the exception set widened (UnknownFiberFormatError, OSError, at minimum) than tightened to one type — or this split into its own PR where the traversal-robustness tradeoff can be argued on its own merits.
There was a problem hiding this comment.
Addressed in 0dca954. Widened to except (UnknownFiberFormatError, OSError) so an unreadable directory (e.g. PermissionError) is simply not treated as a scan unit rather than aborting an index traversal, while genuinely unexpected errors still surface. Kept it in this PR since it sits on the scan path; happy to split it out if you would rather.
| values = coord.values | ||
| if snap or np.ndim(values) != 1: | ||
| return values | ||
| return get_exact_coord(values, units=coord.attrs.get("units")) |
There was a problem hiding this comment.
Minor asymmetry: the snap=False path attaches coord.attrs.get("units"), but the snap=True path returns bare values with no units, so the two modes produce coords that differ in more than exactness. Optodas got the equivalent fix in this PR (get_coord(data=distance, units=unit) on the snap branch); worth doing the same here so snap only controls exactness.
Separately — that optodas units change is itself a user-visible change to snap=True scan output and isn't mentioned in the changelog.
There was a problem hiding this comment.
Addressed in 0dca954. The snap=True NetCDF branch now returns get_coord(data=values, units=...) too, so snap controls only exactness and both modes carry units. The changelog note now also mentions the snap=True units change for OptoDAS, NetCDF, and Terra15.
| time_array = node[time_name] | ||
| array_len = len(time_array) | ||
| assert array_len > 0, "Missing time array in ProdML file." | ||
| if not snap: |
There was a problem hiding this comment.
Worth a docs note: ProdML's read has no snap/snap_dims parameter, so dc.scan_payloads(path, snap=False) returns the exact stored per-sample array while dc.read(path) always returns the header-derived PartStartTime/PartEndTime range. The conformance test in test_common_io.py skips the value comparison for formats without an exact read, so nothing catches the divergence.
That's arguably correct — read is the lossy one — but a caller comparing payload coords to loaded patch coords will be surprised. The snap contract section in new_format.qmd would be a good place to say that snap=False describes stored values, not necessarily what read returns.
There was a problem hiding this comment.
Addressed in 0dca954. Added a paragraph to the snap contract section of new_format.qmd stating that snap=False describes the values a format stores, which are not necessarily what read(...) returns (ProdML reconstructs its time coord from PartStartTime/PartEndTime), and noting the common-IO conformance test only compares scan vs read for formats whose read also accepts snap/snap_dims.
get_exact_coord fell back to CoordSegmented.from_array(tolerance=0) for any monotonic array. Sub-step jitter (e.g. GPS-stamped DAS timestamps) turns nearly every sample into its own seam, producing an n/2-segment coordinate that is slower to build (~120us/segment), larger in memory, and no more exact than a plain monotonic coord. Because Terra15's exact read path also uses this helper, that regressed dc.read(snap_dims=False). - Skip segmentation when a large array would exceed one segment per ten samples, falling back to an exact monotonic/array coordinate; genuinely piecewise-uniform arrays still segment and keep queryable seams. - Promote 0-d input to length-1 so single-sample files (Febus T1) build a valid length-1 coordinate instead of a scalar. - Attach coordinate units on the NetCDF snap=True scan branch so snap only controls exactness, matching OptoDAS. - Widen is_directory_format to also swallow OSError so an unreadable directory is skipped rather than aborting an index traversal. - Document the jitter fallback and the snap=False (stored values) vs read distinction in new_format.qmd; add changelog note.
Add tests for the large non-monotonic fallback in get_exact_coord and for the NetCDF snap=True (units) and multi-dim scan-coord branches so the new diff is fully covered.
This test asserts no ResourceWarning fires while it garbage-collects its own spool. But gc.collect() also reaps collectable garbage left by earlier tests, and a leaked open SQLite connection emits a ResourceWarning when collected -- which the recording window then caught and blamed on this test. The result was a full-suite-order failure (reproducible on dev independent of this branch; #754 merged with this same test_code job red) that passed in isolation and within its own file. Drain pending garbage with a gc.collect() before opening the recording window, so only the spool under test is collected inside it. The test's intent -- this spool's backend connection closes silently on GC -- is unchanged.
1 participant
Description
dc.scan_payloads(...)as a sibling todc.scan(...), returning rawScanPayloaddictionaries with fullCoordManagerobjects and dispatcher-owned source provenanceFiberIO.scan(..., snap: bool = True)while keepingdc.scan(...)anddc.scan_to_df(...)output and signatures unchangedsnap=Falsepreserve stored coordinate values through zero-toleranceCoordSegmented.from_array(...)construction in stored-array formatsFormat audit
snap=FalsebehaviorDasTimeArrayReview notes
tolerance=0. This keeps every stored value unchanged while still returningCoordRangefor truly uniform arrays andCoordSegmentedfor exact monotonic runs separated by seams.snap/snap_dimsnaming remains a follow-up.exact_scan_coordsremains deferred until planner integration needs it.Testing
pre-commit run --allpytest tests -q— 7,368 passed, 87 skipped, 3 xfailedChangelog
dc.scan_payloads(...)returns raw scan payloads with full coordinate managers and source provenance without loading data arrays.FiberIO.scan(...)standardizessnap: bool = True;snap=Falserequests exact stored coordinate values, and a jittery per-sample array falls back to a plain monotonic coordinate rather than exploding into per-sample segments.snap=True) scan and read output — Terra15 exact times, OptoDAS and NetCDF distances.ScanPayloadcarries optionalsource_path,source_format, andsource_version, populated bydc.scan_payloads(...)rather than by format implementations.Checklist
I have (if applicable):
Summary by CodeRabbit
New Features
scan_payloads(...)to retrieve raw scan payloads with fullCoordManagercoordinates and optional source provenance (source_path/source_format/source_version).User-Facing Changes
scan(..., snap: bool = True)across supported formats;snap=Falserequests exact stored per-sample coordinate values when available.Documentation
snapvs exact-coordinate behavior.Tests
snap=Falseexact-value conformance.