Skip to content

Verasonics multibuffer - #538

Draft
lvknippenberg wants to merge 5 commits into
tue-bmd:mainfrom
lvknippenberg:verasonics-multibuffer-clean
Draft

Verasonics multibuffer#538
lvknippenberg wants to merge 5 commits into
tue-bmd:mainfrom
lvknippenberg:verasonics-multibuffer-clean

Conversation

@lvknippenberg

@lvknippenberg lvknippenberg commented Jul 31, 2026

Copy link
Copy Markdown

[Feature]: Verasonics converter — multi-buffer RF + external .bin RF support

Wessel mentioned that my fast saving method (dumping RF data in .bin files) and support for multiple RF buffers could be valuable additions to the toolbox. Please let me know what you think and what changes would be required.

What changed?

Extends the Verasonics converter (zea/data/convert/verasonics.py) to convert
acquisitions with multiple RF buffers and to read RF from external binary
files
, while keeping all existing single-buffer behaviour intact. Adds
standalone tests and documentation.

  • Multiple RF buffers. A single .mat whose RcvData holds more than one
    cell is now convertible one buffer at a time. to_zea(buffer_index=...)
    selects a buffer; to_zea_all() converts every buffer from one open file
    (the shared Receive/Event reads are memoised, so the cost is paid once).
    The RF image buffer (ImgDataP / Resource.ImageBuffer) is indexed
    independently via image_buffer_index.
  • External .bin RF. For large acquisitions the RF of each buffer can be
    saved to a raw int16 file RF_data_{k}.bin next to a metadata .mat that
    stores the per-buffer dimensions (RF_rows, RF_cols, RF_frames) and the
    saved-channel masks (NonzeroRFcolumns). The converter reconstructs the
    full-channel array from these, so everything downstream is identical to the
    in-.mat path.
  • CLI dispatch. A source directory containing RF_data_*.bin is detected as
    one multi-buffer acquisition and converted per buffer; a directory of plain
    .mat files keeps the existing one-file-per-.mat behaviour. The metadata
    .mat may have any name (exactly one must be present).
  • Multi-TGC files. Acquisitions that store one TGC waveform per buffer
    (TGC.Waveform as an array of references) are handled by selecting the
    waveform each buffer's receives use (Receive.TGC). Single-TGC files
    (plain matrix) are unchanged.
  • Robust reference detection. Cell / struct-array datasets are detected via
    h5py.check_dtype(ref=...) instead of inspecting fillvalue, which is only a
    null reference for writers that set it explicitly (MATLAB does). No behaviour
    change on real MATLAB files; also makes the read paths unit-testable with
    synthetic fixtures.
  • Robust channel selection. Trans.ConnectorES is applied whenever the
    acquired channel count differs from the element count (previously only for the
    updated save_raw format), fixing an n_el mismatch that also affected some
    single-buffer files.
  • Windows logging fix (zea/log.py). The console stream is reconfigured
    with errors="backslashreplace" so Unicode log characters (arrows,
    checkmarks) don't raise UnicodeEncodeError on legacy-codepage (cp1252)
    consoles.
  • Tests (tests/test_verasonics_multibuffer.py). New standalone unit tests
    (no external data) that build tiny synthetic MATLAB-v7.3 HDF5 workspaces in
    tmp_path and verify per-buffer RF reading for both storage forms.
  • Docs (docs/source/data-acquisition.rst). The Verasonics section now
    documents the multi-buffer API and the external .bin workflow, including the
    on-disk format contract and a vendored reference MATLAB saver
    (docs/source/_examples/WriteRFbuffersBin.m) shown via literalinclude.

Why?

Cardiac ARF / shear-wave acquisitions produce several RF buffers per recording
(e.g. a wide-beam B-mode buffer plus an active shear-wave tracking buffer, or
subsequent measurements such as imaging then Doppler), and writing the full
RcvData to a v7.3 .mat is prohibitively slow for large buffers — hence the
external .bin workflow. The previous converter could only handle a single
in-.mat buffer, and tripped over per-buffer TGC waveforms and an 80-vs-128
channel mismatch on real probes.

How it addresses the issue

Buffer selection is threaded through the read path (read_transmit_events,
read_scan, read_raw_data, sampling/demod/TGC accessors) via a 0-based
buffer_index, resolved from Receive.bufnum. read_raw_buffer_array is the
single entry point that returns one buffer as (n_frames, n_channels, n_samples)
from either RcvData or RF_data_{k}.bin, so the rest of the pipeline is
storage-agnostic.

Files

  • zea/data/convert/verasonics.py — converter changes.
  • zea/log.py — Windows Unicode logging fix.
  • tests/test_verasonics_multibuffer.py — new standalone unit tests.
  • docs/source/data-acquisition.rst — Verasonics multi-buffer / .bin docs.
  • docs/source/_examples/WriteRFbuffersBin.m — vendored reference MATLAB saver.
    (Placement is provisional — happy to move it to an examples/ tree or drop
    it if maintainers prefer; see "Open questions" below.)

Testing

Standalone unit tests in tests/test_verasonics_multibuffer.py:

  • multi-cell RcvData: buffer enumeration, Receive.bufnum → buffer mapping,
    correct per-buffer read;
  • external .bin: full-channel reconstruction via NonzeroRFcolumns,
    "defined but not saved" buffers, dimension-mismatch error.

Run:

pip install -e .[dev]
pre-commit run --all-files
pytest -m 'not heavy and not notebook' --skip-unavailable-backends
# targeted:
pytest tests/test_verasonics_multibuffer.py tests/test_verasonics_geometry.py \
       tests/test_log.py tests/data/test_convert_main.py --skip-unavailable-backends
# real-file regression (heavy):
pytest "tests/data/test_conversion_scripts.py::test_conversion_script[verasonics]"

Locally: ruff check, ruff format --check, and ty check are clean on the
changed files; the targeted tests and the heavy
test_conversion_script[verasonics] pass (confirming the robust reference
detection does not change behaviour on real MATLAB files). The docs section was
rendered with an isolated Sphinx build (sphinx_design, -W --keep-going): no
warnings; the dropdowns, matlab code blocks and the literalinclude all
render.

Also verified end-to-end on a real 2-buffer acquisition saved both ways
(external .bin vs conventional .mat): the converted RF matches between the
two (identical shapes, dtypes, sampling frequency and n_tx; near-identical
values), including the baseband-IQ buffer.

Open questions for maintainers

  • Where should the reference MATLAB saver live? It is currently under
    docs/source/_examples/ and pulled into the docs with literalinclude. An
    examples/verasonics/ tree (or omitting the file and keeping only the format
    contract in the docs) are equally fine — happy to move it.

Summary by CodeRabbit

  • New Features

    • Added support for converting Verasonics acquisitions containing multiple RF buffers.
    • Added support for externally stored per-buffer binary RF data, including channel-mask restoration and zero-filled channels.
    • Added options to select RF and image buffers independently.
    • Added conversion of individual buffers or all buffers with configurable overwrite and error handling.
  • Documentation

    • Added guidance for multi-buffer conversion, external binary layouts, metadata requirements, and MATLAB acquisition callbacks.

lvknippenberg and others added 5 commits July 30, 2026 22:04
Adds support for converting Verasonics acquisitions with multiple RF
buffers and image buffers on top of the current main converter, keeping
all existing single-file / directory-of-.mat functionality intact.

verasonics.py:
- Read a single RF buffer selected by bufnum (RcvData{k}), with per-buffer
  accessors for sampling frequency, sample mode, demod frequency, n_ax,
  start/end samples and TGC.
- Support RF stored in external RF_data_{k}.bin files (int16, column-major)
  alongside a metadata .mat, reconstructed via RF_rows/RF_cols/RF_frames
  and NonzeroRFcolumns.
- Independent image-buffer selection (ImgDataP / Resource.ImageBuffer).
- to_zea(buffer_index, image_buffer_index) and to_zea_all() to convert
  every buffer from a single open file (shared Receive/Event reads are
  memoised, so the per-buffer cost is paid once).
- Vectorised read_transmit_events and cached scalar-field reads for large
  Event/Receive structures; optional ZEA_VERASONICS_PROFILE timing.
- Robust channel selection via Trans.ConnectorES whenever the acquired
  channel count differs from the element count (fixes an n_el mismatch
  that also affected single-buffer files).
- CLI: a source directory containing RF_data_*.bin is detected as one
  multi-buffer acquisition and dispatched to to_zea_all; a directory of
  plain .mat files keeps the one-file-per-.mat behaviour.

log.py:
- Reconfigure the console stream with errors="backslashreplace" so Unicode
  log characters (arrows, checkmarks) don't raise UnicodeEncodeError on
  Windows legacy-codepage consoles.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Handle files that store one TGC waveform per RF buffer. Such files store
  TGC.Waveform as an array of HDF5 references (one per TGC struct) rather than
  a plain matrix; the correct waveform for a buffer is selected via the 1-based
  Receive.TGC field. Single-TGC files (plain matrix) are unchanged. Fixes a
  "TypeError: unsupported operand /: Reference and int" on multi-buffer
  acquisitions.
- Remove the ZEA_VERASONICS_PROFILE timing instrumentation (the _profile
  context manager and its call sites), keeping only the functional
  optimizations (cached scalar-field reads, vectorised event parsing, etc.).

Verified end to end on a custom .bin multi-buffer acquisition and its
conventional single-.mat counterpart: both RF buffers convert, and the
resulting RF matches between the two saving methods (identical shapes,
dtypes, sampling frequency and n_tx; near-identical values), including the
baseband-IQ buffer.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Apply ruff-format (v0.15.22) to verasonics.py (line-wrapping only; no logic
  change).
- Make the log.py console-stream reconfigure type-safe: narrow via
  isinstance(stream, io.TextIOWrapper) instead of hasattr, so `ty` no longer
  reports call-non-callable on stream.reconfigure(); use contextlib.suppress.

Verification (all green): ruff check, ruff format --check, ty check on both
files; pytest tests/test_log.py, tests/test_verasonics_geometry.py,
tests/data/test_convert_main.py, the verasonics tests in
tests/data/test_conversion_scripts.py, and the heavy
test_conversion_script[verasonics] full-conversion test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tection

- tests/test_verasonics_multibuffer.py: standalone unit tests (no external
  data) that build tiny synthetic MATLAB-v7.3 HDF5 workspaces in tmp_path and
  verify per-buffer RF reading for both storage forms:
    * an in-.mat multi-cell RcvData (buffer enumeration, Receive.bufnum ->
      buffer mapping, correct per-buffer read),
    * external RF_data_{k}.bin files (full-channel reconstruction via
      NonzeroRFcolumns, "defined but not saved" buffers, dimension-mismatch
      error).
- verasonics.py: detect reference (cell/struct-array) datasets via
  h5py.check_dtype(ref=...) in a single _is_reference_dataset helper instead of
  isinstance(dataset.fillvalue, h5py.h5r.Reference). check_dtype is robust
  across HDF5 writers; fillvalue is only a null reference when the writer sets
  it (MATLAB does, h5py's high-level API does not), which also makes the read
  paths unit-testable with synthetic fixtures. Behaviour on real MATLAB files
  is unchanged (verified by the heavy test_conversion_script[verasonics]).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extend the Verasonics section of the data-acquisition docs with:
- converting multiple RF buffers (to_zea(buffer_index=...), to_zea_all), and
  the independent RF/image buffer indexing;
- the external RF_data_{k}.bin workflow: CLI auto-detection of a measurement
  directory, the on-disk format contract (RF_rows/RF_cols/RF_frames,
  NonzeroRFcolumns, int16 column-major, no RcvData in the .mat), and the
  Verasonics save-button wiring;
- a vendored reference MATLAB saver, docs/source/_examples/WriteRFbuffersBin.m,
  shown via literalinclude.

Rendering verified with an isolated sphinx build (sphinx_design) with
-W --keep-going: no warnings; dropdowns, matlab code-blocks and the
literalinclude all render.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

Verasonics multi-buffer conversion

Layer / File(s) Summary
External buffer storage contract
docs/source/_examples/WriteRFbuffersBin.m, docs/source/data-acquisition.rst, tests/test_verasonics_multibuffer.py
Adds MATLAB export of per-buffer RF binaries and metadata. Documents the binary layout and creates test fixtures.
Buffer-aware metadata and event processing
zea/data/convert/verasonics.py
Adds buffer-specific reference reads, event filtering, timing, sampling, demodulation, sample mode, baseband, and TGC handling.
Raw buffer reconstruction and channel mapping
zea/data/convert/verasonics.py, tests/test_verasonics_multibuffer.py
Reconstructs buffers from in-file or external data, restores omitted channels, applies connector mappings, and validates file dimensions.
Independent image conversion and dispatch
zea/data/convert/verasonics.py
Adds independent image-buffer selection, to_zea_all, external-buffer CLI dispatch, probe fallbacks, and per-buffer conversion handling.
Standard output preparation
zea/log.py
Configures standard text output to replace unencodable characters while preserving redirected stream behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

  • tue-bmd/zea#188: Introduced the Verasonics conversion APIs extended here for multi-buffer support.
  • tue-bmd/zea#457: Changed related read_verasonics_file() and to_zea() lens and baseband-IQ handling.

Suggested reviewers: wesselvannierop

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: multi-buffer support for the Verasonics converter.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
docs/source/_examples/WriteRFbuffersBin.m (1)

119-126: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid a full in-memory copy of each RF buffer before fwrite.

RcvData{k}(:, NonzeroRFcolumns{k}, :) builds a complete second copy of the buffer. For the large buffers this script targets, peak memory doubles. Write frame by frame instead, and skip the indexing when every channel is saved.

♻️ Proposed per-frame write
     FN = fullfile(save_folder_path, sprintf('RF_data_%d.bin', k));
     [fid, msg] = fopen(FN, 'w');
     if fid < 0
         error('WriteRFbuffersBin:fopen', 'Could not open %s for writing: %s', FN, msg);
     end
-    fwrite(fid, RcvData{k}(:, NonzeroRFcolumns{k}, :), 'int16');
+    allChannels = all(NonzeroRFcolumns{k});
+    for frame = 1:RF_frames(k)
+        if allChannels
+            fwrite(fid, RcvData{k}(:, :, frame), 'int16');
+        else
+            fwrite(fid, RcvData{k}(:, NonzeroRFcolumns{k}, frame), 'int16');
+        end
+    end
     fclose(fid);
🤖 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 `@docs/source/_examples/WriteRFbuffersBin.m` around lines 119 - 126, Update the
write loop around RcvData and fwrite to avoid materializing the full indexed RF
buffer: write the data frame by frame, applying NonzeroRFcolumns{k} to each
frame before fwrite. When all channels are selected, write each frame directly
without indexing, while preserving the existing file-opening, int16 output, and
cleanup behavior.
zea/data/convert/verasonics.py (1)

1348-1354: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Rename the sampled-index local to avoid the probe collision.

probe here holds the spot-check indices. In this class, self.probe is the VerasonicsProbe transducer object. Use a name such as check_indices in both methods.

♻️ Proposed rename
-        probe = self._subset_indices(indices, self.CONSTANT_FIELD_CHECK_SAMPLES)
+        check_indices = self._subset_indices(indices, self.CONSTANT_FIELD_CHECK_SAMPLES)
         values = np.array(
             [
                 float(np.asarray(self.dereference_index(dataset, int(i))).reshape(-1)[0])
-                for i in probe
+                for i in check_indices
             ]
         )

Also applies to: 1542-1546

🤖 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 `@zea/data/convert/verasonics.py` around lines 1348 - 1354, Rename the local
variable `probe` to `check_indices` in the code block where it holds the sampled
spot-check indices returned from _subset_indices. This avoids the naming
collision with the class attribute self.probe which is the VerasonicsProbe
transducer object. Apply this same rename to the other occurrence mentioned at
lines 1542-1546, ensuring consistent naming across both methods.
🤖 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 `@zea/data/convert/verasonics.py`:
- Around line 2492-2498: The to_zea_all call in the multibuffer conversion path
must not unconditionally pass overwrite=True. Reuse the existing get_answer and
existing_file_policy flow used by the ordinary file-pair path, and only enable
overwriting after the user explicitly allows replacing existing {stem}_buffer
outputs.
- Around line 650-663: Guard the frame indexing in the time_to_next_acq handling
after reshaping and before applying frame_indices. Only index when the reshaped
array has rows and all frame_indices are within its row bounds; otherwise set
time_to_next_acq to None, preserving the existing drop-timing behavior for empty
or mismatched event-frame data.

---

Nitpick comments:
In `@docs/source/_examples/WriteRFbuffersBin.m`:
- Around line 119-126: Update the write loop around RcvData and fwrite to avoid
materializing the full indexed RF buffer: write the data frame by frame,
applying NonzeroRFcolumns{k} to each frame before fwrite. When all channels are
selected, write each frame directly without indexing, while preserving the
existing file-opening, int16 output, and cleanup behavior.

In `@zea/data/convert/verasonics.py`:
- Around line 1348-1354: Rename the local variable `probe` to `check_indices` in
the code block where it holds the sampled spot-check indices returned from
_subset_indices. This avoids the naming collision with the class attribute
self.probe which is the VerasonicsProbe transducer object. Apply this same
rename to the other occurrence mentioned at lines 1542-1546, ensuring consistent
naming across both methods.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a0487fe3-f85d-46c9-a620-14b13360cf08

📥 Commits

Reviewing files that changed from the base of the PR and between 56de3e0 and 8c2699f.

📒 Files selected for processing (5)
  • docs/source/_examples/WriteRFbuffersBin.m
  • docs/source/data-acquisition.rst
  • tests/test_verasonics_multibuffer.py
  • zea/data/convert/verasonics.py
  • zea/log.py

Comment on lines 650 to 663
if time_to_next_acq is not None:
time_to_next_acq = time_to_next_acq[frame_indices]
n_tx = tx_order.size
if n_tx > 0 and time_to_next_acq.size % n_tx == 0:
time_to_next_acq = np.reshape(time_to_next_acq, (-1, n_tx))
time_to_next_acq = time_to_next_acq[frame_indices]
else:
log.warning(
"Could not reshape time_to_next_acq "
f"({time_to_next_acq.size} values) into frames of {n_tx} "
"transmits; the transmit structure is not uniform across "
"frames for this buffer. Dropping time-to-next-acquisition."
)
time_to_next_acq = None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Guard the frame indexing after the reshape.

frame_indices is bounded by Resource.RcvBuffer.numFrames, not by the number of timing rows produced here. The reshaped array has one row per event frame found for this buffer, and it has zero rows when the sequence contains no timeToNextAcq command. In both cases line 654 raises IndexError and aborts the conversion, although the surrounding code already prefers to drop the timing.

🛡️ Proposed bounds guard
         if time_to_next_acq is not None:
             n_tx = tx_order.size
-            if n_tx > 0 and time_to_next_acq.size % n_tx == 0:
+            if n_tx > 0 and time_to_next_acq.size >= n_tx and time_to_next_acq.size % n_tx == 0:
                 time_to_next_acq = np.reshape(time_to_next_acq, (-1, n_tx))
-                time_to_next_acq = time_to_next_acq[frame_indices]
+                in_range = frame_indices[frame_indices < time_to_next_acq.shape[0]]
+                if in_range.size != frame_indices.size:
+                    log.warning(
+                        f"Only {time_to_next_acq.shape[0]} timing frame(s) are available "
+                        f"for buffer_index={buffer_index}; requested frames outside that "
+                        "range are dropped from time_to_next_transmit."
+                    )
+                time_to_next_acq = time_to_next_acq[in_range]
             else:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if time_to_next_acq is not None:
time_to_next_acq = time_to_next_acq[frame_indices]
n_tx = tx_order.size
if n_tx > 0 and time_to_next_acq.size % n_tx == 0:
time_to_next_acq = np.reshape(time_to_next_acq, (-1, n_tx))
time_to_next_acq = time_to_next_acq[frame_indices]
else:
log.warning(
"Could not reshape time_to_next_acq "
f"({time_to_next_acq.size} values) into frames of {n_tx} "
"transmits; the transmit structure is not uniform across "
"frames for this buffer. Dropping time-to-next-acquisition."
)
time_to_next_acq = None
if time_to_next_acq is not None:
n_tx = tx_order.size
if n_tx > 0 and time_to_next_acq.size >= n_tx and time_to_next_acq.size % n_tx == 0:
time_to_next_acq = np.reshape(time_to_next_acq, (-1, n_tx))
in_range = frame_indices[frame_indices < time_to_next_acq.shape[0]]
if in_range.size != frame_indices.size:
log.warning(
f"Only {time_to_next_acq.shape[0]} timing frame(s) are available "
f"for buffer_index={buffer_index}; requested frames outside that "
"range are dropped from time_to_next_transmit."
)
time_to_next_acq = time_to_next_acq[in_range]
else:
log.warning(
"Could not reshape time_to_next_acq "
f"({time_to_next_acq.size} values) into frames of {n_tx} "
"transmits; the transmit structure is not uniform across "
"frames for this buffer. Dropping time-to-next-acquisition."
)
time_to_next_acq = None
🤖 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 `@zea/data/convert/verasonics.py` around lines 650 - 663, Guard the frame
indexing in the time_to_next_acq handling after reshaping and before applying
frame_indices. Only index when the reshaped array has rows and all frame_indices
are within its row bounds; otherwise set time_to_next_acq to None, preserving
the existing drop-timing behavior for empty or mismatched event-frame data.

Comment on lines +2492 to +2498
with VerasonicsFile(multibuffer_mat, "r") as file:
results = file.to_zea_all(
output_path,
overwrite=True,
frames=args.frames,
allow_accumulate=args.allow_accumulate,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Do not overwrite existing buffer outputs without asking.

The ordinary file-pair path prompts through get_answer and honours existing_file_policy before it deletes an output. This path passes overwrite=True, so a second run of the same command silently deletes every previously converted {stem}_buffer{k}.hdf5. Reuse the same policy, or gate the overwrite behind an explicit answer.

🛡️ Proposed prompt-based overwrite
         with VerasonicsFile(multibuffer_mat, "r") as file:
+            overwrite_existing = True
+            existing = sorted(Path(output_path).glob("*_buffer*.hdf5"))
+            if existing:
+                overwrite_existing = get_answer(
+                    f"{len(existing)} converted buffer file(s) already exist in "
+                    f"{log.yellow(output_path)}. Overwrite? (y/n): "
+                )
             results = file.to_zea_all(
                 output_path,
-                overwrite=True,
+                overwrite=overwrite_existing,
                 frames=args.frames,
                 allow_accumulate=args.allow_accumulate,
             )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
with VerasonicsFile(multibuffer_mat, "r") as file:
results = file.to_zea_all(
output_path,
overwrite=True,
frames=args.frames,
allow_accumulate=args.allow_accumulate,
)
with VerasonicsFile(multibuffer_mat, "r") as file:
overwrite_existing = True
existing = sorted(Path(output_path).glob("*_buffer*.hdf5"))
if existing:
overwrite_existing = get_answer(
f"{len(existing)} converted buffer file(s) already exist in "
f"{log.yellow(output_path)}. Overwrite? (y/n): "
)
results = file.to_zea_all(
output_path,
overwrite=overwrite_existing,
frames=args.frames,
allow_accumulate=args.allow_accumulate,
)
🤖 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 `@zea/data/convert/verasonics.py` around lines 2492 - 2498, The to_zea_all call
in the multibuffer conversion path must not unconditionally pass overwrite=True.
Reuse the existing get_answer and existing_file_policy flow used by the ordinary
file-pair path, and only enable overwriting after the user explicitly allows
replacing existing {stem}_buffer outputs.

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 61.94030% with 153 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
zea/data/convert/verasonics.py 61.61% 111 Missing and 41 partials ⚠️
zea/log.py 83.33% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Comment on lines +1 to +6
%WriteRFbuffersBin Save Verasonics RF buffers as raw .bin files + one .mat.
%
% Replaces the slow '-v7.3' save of the full RcvData: each RF buffer is written
% to a raw int16 binary file, and only the (small) dimensions and metadata
% needed to reconstruct it are stored in a single Parameters.mat next to the
% binaries.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to mention we keep zea python-only, so while the compatibility of the conversion script with bin files seems nice to me. This might be a better fit for our (internal): https://github.com/tue-bmd/verascripts

@wesselvannierop wesselvannierop left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will review the rest later!

@wesselvannierop wesselvannierop self-assigned this Aug 7, 2026
@wesselvannierop wesselvannierop added the data format Related to the zea data format saving and loading label Aug 7, 2026
@tristan-deep tristan-deep added this to the v0.1.6 milestone Aug 11, 2026
@tristan-deep
tristan-deep marked this pull request as draft August 12, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data format Related to the zea data format saving and loading

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants