dascore/data_registry.txt (1)

33-33: Checksum Verified – Asset Integrity Confirmed

• The SHA256 of sintela_binary_v3_test_1.raw matches the declared 4c0afae1ab60b73ddcb3c4f7ac55b976d4d18d1ffc9ea6cd2dfa6a881b697101.
• Consider adding a mirror URL alongside the primary GitHub link for improved availability.
• It’s still recommended (as a CI enhancement) to verify the SHA256 locally on first download to guard against silent upstream changes.

File: dascore/data_registry.txt (line 33)

dascore/constants.py (1)

66-68: New data types align with Sintela v3 – ensure consistent casing across readers

Adding "phase_difference" and "phase_rate" is great. Please confirm all producers/consumers use lowercase values; in the new Sintela utils, "Strain" and "Strain Rate" appear with capitalization which will not match these constants.

Would you like me to generate a quick grep to find inconsistent data_type strings across the repo?

dascore/io/sintela_binary/utils.py (5)

102-106: Avoid shadowing imported name ‘array’

Local variable array shadows the imported function array from dascore.compat. Rename for clarity.

 def _read_base_header(fid):
     """Return the first 3 elements of the sintela header."""
-    array = np.fromfile(fid, dtype=base_header_dtypes, count=1)
-    out = {x: y for x, y in zip(array.dtype.names, array[0])}
+    buf = np.fromfile(fid, dtype=base_header_dtypes, count=1)
+    out = {x: y for x, y in zip(buf.dtype.names, buf[0])}
     return out

---

120-131: Prefer explicit exceptions over asserts for header validation

Asserts may be stripped under optimization and are not user-friendly. Raise a clear exception if header sizes don’t match.

-    expected_size = _HEADER_SIZES.get(version, -1)
-    assert header_size == expected_size
+    expected_size = _HEADER_SIZES.get(version, -1)
+    if header_size != expected_size:
+        raise ValueError(
+            f"Header size {header_size} does not match expected {expected_size} "
+            f"for version {version}."
+        )

---

141-149: Guard against invalid sample_rate and ensure timedelta conversion is supported

If sample_rate is 0/NaN or extremely small, this will raise/overflow. Also ensure dc.to_timedelta64(float) is supported.

-    timestep = dc.to_timedelta64(1 / header["sample_rate"])
+    sr = float(header["sample_rate"])
+    if not np.isfinite(sr) or sr <= 0:
+        raise ValueError(f"Invalid sample_rate: {sr}")
+    timestep = dc.to_timedelta64(1 / sr)

If to_timedelta64 does not support floats in your environment, I can switch to np.timedelta64(int(round(1e9/sr)), "ns") to avoid dependency on that helper.

---

152-166: Confirm channel indexing convention (off-by-one risk)

Distance start is start_channel * dx. If start_channel is 1-based in Sintela v3, this will shift distance by one channel (should be (start_channel - 1) * dx). Please confirm against the v3 spec or sample header.

I can adapt this function once we verify the convention.

---

203-211: Attr construction: let PatchAttrs derive dims from CoordManager

Minor: since you pass cm into attrs, you can drop explicit dims injection earlier; PatchAttrs can derive dims from coords. This reduces duplication and mismatch risk.

No code change required here if you apply the earlier change in _get_attr_dict to omit dims.

dascore/io/sintela_binary/core.py (3)

23-28: Correct class and module docstrings; fix copy/paste artifacts

• "Patch Attributes for Silixa hdf5 format." and module references to Silixa/Terra15 are misleading here.

 class SintelaPatchAttrs(dc.PatchAttrs):
-    """Patch Attributes for Silixa hdf5 format."""
+    """Patch Attributes for Sintela binary format."""

I’ve included broader docstring fixes below for get_format/scan/read.

---

37-55: Update get_format docstring and parameter description

Docstrings still mention Silixa/Terra15.

     def get_format(self, resource: BinaryReader, **kwargs) -> tuple[str, str] | bool:
         """
-        Return name and version string if Silixa hdf5 else False.
+        Return (name, version) if this is a Sintela v3 binary file, else False.
 
         Parameters
         ----------
         resource
-            A path to the file which may contain terra15 data.
+            A path or binary stream to the candidate Sintela file.
         """

---

56-66: scan docstring mentions Silixa; also ensure resource offset reset

Minor docstring fix. The BinaryReader type-caster resets the offset, so this is fine functionally.

-    def scan(self, resource: BinaryReader, **kwargs) -> list[dc.PatchAttrs]:
-        """Scan a Silixa HDF5 file, return summary information on the contents."""
+    def scan(self, resource: BinaryReader, **kwargs) -> list[dc.PatchAttrs]:
+        """Scan a Sintela v3 binary file and return summary attributes."""

dascore/io/sintela_binary/__init__.py (2)

4-4: Explicitly export re-exported symbol to satisfy Ruff F401 and make API intent clear

SintelaBinaryV3 is re-exported for public use, but Ruff flags it as "imported but unused". Declare __all__ so the intent is explicit and the lint warning is silenced.

Apply this diff:

 """
 Sintela binary reader.
 """
 from .core import SintelaBinaryV3
+
+__all__ = ["SintelaBinaryV3"]

---

1-3: Optional: clarify the docstring to mention the supported version

Minor clarity win since this package may eventually host multiple versions.

-"""
-Sintela binary reader.
-"""
+"""
+Sintela binary reader (version 3).
+"""

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

pyproject.toml (1)

123-123: Entry point looks correct; ensure public re-export path exists

SINTELA_BINARY__V3 mapping is good. Please confirm dascore/io/sintela_binary/__init__.py re-exports SintelaBinaryV3 so from dascore.io.sintela_binary import SintelaBinaryV3 works (tests rely on this pattern).

tests/test_io/test_common_io.py (1)

39-39: LGTM: importing from the package namespace keeps tests aligned with the public API

This import ensures tests use the stable, documented path rather than reaching into module internals. No changes needed.

dascore/utils/misc.py (1)

751-769: Clarify return type and harden size detection (fd-backed streams, SEEK constants).

• Add a return annotation (int).
• Use os.SEEK_* for readability/portability.
• If fid.name is not a path-like (eg, some temp files expose an int fd), Path(path) will raise TypeError. Fall back to os.fstat(fid.fileno()).st_size.

Apply this diff:

-def get_buffer_size(fid: IOBase):
+def get_buffer_size(fid: IOBase) -> int:
@@
-    path = getattr(fid, "name", None)
-    if path is None:
-        cur = fid.tell()
-        fid.seek(0, 2)  # end
-        file_size = fid.tell()
-        fid.seek(cur, 0)
-    else:
-        file_size = Path(path).stat().st_size
-    return file_size
+    path = getattr(fid, "name", None)
+    if path is None:
+        cur = fid.tell()
+        fid.seek(0, os.SEEK_END)
+        file_size = fid.tell()
+        fid.seek(cur, os.SEEK_SET)
+        return file_size
+    # If `name` isn't a filesystem path (eg, int fd), fall back to fstat.
+    try:
+        return Path(path).stat().st_size
+    except TypeError:
+        return os.fstat(fid.fileno()).st_size

tests/test_utils/test_misc.py (2)

357-362: Good type assertion for file-backed memmap.

Asserts np.memmap is returned for real files. Consider also asserting array.size > 0 to guard against empty mappings.

---

363-370: Add a test for non-zero file pointer to catch partial reads in fallback.

Currently, if fid.tell() != 0, the fallback path can return a truncated array. Add a test that writes without seeking back to 0 before calling maybe_mem_map. This will safeguard the intended semantics once the fix lands.

Proposed test addition:

 class TestMaybeMemMap:
@@
     def test_bytes_io(self):
         """Ensure non-files return arrays."""
         bio = BytesIO()
         bio.write(b"1234")
         bio.seek(0)
         array = maybe_mem_map(bio)
         assert isinstance(array, np.ndarray)
+    
+    def test_bytes_io_nonzero_position(self):
+        """Fallback should read entire buffer even if pointer is not at 0."""
+        bio = BytesIO()
+        bio.write(b"abcdef")
+        # Intentionally do not seek back to 0; pointer is at end.
+        arr_end_pos = maybe_mem_map(bio)
+        # Now reset to start and compare lengths; both should see full content.
+        bio.seek(0)
+        arr_full = maybe_mem_map(bio)
+        assert arr_end_pos.size == arr_full.size == 6

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

dascore/utils/misc.py (1)

14-14: Import of IOBase is appropriate for public APIs working with file-like objects.

Good call surfacing IOBase for type annotations on the new helpers.

tests/test_utils/test_misc.py (2)

334-343: Solid coverage for file-backed size detection.

Validates parity with filesystem stat; this anchors behavior for regular files.

---

344-352: Nice BytesIO coverage and pointer preservation check.

Confirms size is stable regardless of position; aligns with the helper’s contract.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.