Skip to content

fix(sinks): slice list and tuple custom_data values per row#2216

Merged
Borda merged 12 commits intoroboflow:developfrom
shaun0927:fix/sinks-slice-list-values
Apr 17, 2026
Merged

fix(sinks): slice list and tuple custom_data values per row#2216
Borda merged 12 commits intoroboflow:developfrom
shaun0927:fix/sinks-slice-list-values

Conversation

@shaun0927
Copy link
Copy Markdown
Contributor

Before submitting
  • Self-reviewed the code
  • Updated documentation, follow Google-style
  • Added docs entry for autogeneration (if new functions/classes)
  • Added/updated tests
  • All tests pass locally

Description

Extend the per-row slicing already done for numpy.ndarray values in CSVSink and JSONSink so it also covers plain Python list / tuple values whose length matches the number of detections. Shorter or differently-sized sequences keep the current broadcast behavior, so existing complex-data tests continue to pass.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)

Motivation and Context

#2199 fixed the "entire array on every row" bug for numpy.ndarray values in custom_data, but the patch only added a numpy.ndarray branch to _slice_value; any non-ndarray value still returned unchanged. When a user passes a plain Python list — e.g., per-detection string ids — every row receives the full list instead of its per-row element:

import numpy as np
import supervision as sv

det = sv.Detections(
    xyxy=np.array([[0, 0, 10, 10], [20, 20, 40, 40], [50, 50, 70, 70]]),
    class_id=np.array([1, 2, 3]),
    confidence=np.array([0.9, 0.8, 0.7]),
)
with sv.CSVSink("out.csv") as sink:
    sink.append(det, custom_data={"ids": ["a", "b", "c"]})

Before this PR the ids column contains "['a', 'b', 'c']" on every row; with the fix each row gets "a", "b", "c" respectively. The same regression applies to JSONSink.

The complex_data test cases in tests/detection/test_csv.py ({"tags": ["urgent", "review"]} broadcast across a single detection) still pass because the length check guarantees the existing broadcast behavior whenever the sequence length does not match the detection count.

Changes Made

  • src/supervision/detection/tools/csv_sink.py_slice_value now also indexes list / tuple values whose length equals the number of detections; other non-array values are still broadcast.
  • src/supervision/detection/tools/json_sink.py — same change, keeping parity between the two sinks.
  • tests/detection/test_csv.py — new test_csv_sink parameter covering a 3-row, 2-row, and 1-row sink over list and tuple custom data.
  • tests/detection/test_json.py — equivalent new parameter for test_json_sink.

Testing

  • I have tested this code locally
  • I have added unit tests that prove my fix is effective or that my feature works
  • All new and existing tests pass

Google Colab (optional)

Screenshots/Videos (optional)

Additional Notes

Backward compatibility: scalar values ({"frame_number": 42}) and sequences whose length does not match the number of detections ({"tags": ["urgent", "review"]} against a single detection) still write unchanged to every row, matching the behavior exercised by the existing complex_data tests in both sinks.

Extends the per-row slicing already done for numpy arrays in CSVSink
and JSONSink to also cover plain Python list and tuple values whose
length matches the number of detections. Shorter or differently-sized
sequences are still broadcast unchanged to every row to preserve the
existing complex-data behaviour.
@shaun0927 shaun0927 requested a review from SkalskiP as a code owner April 17, 2026 04:29
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 17, 2026

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77%. Comparing base (5d165e0) to head (a3564a1).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #2216   +/-   ##
=======================================
  Coverage       77%     77%           
=======================================
  Files           66      66           
  Lines         8189    8195    +6     
=======================================
+ Hits          6303    6309    +6     
  Misses        1886    1886           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes per-detection row serialization in CSVSink and JSONSink so that custom_data values provided as plain Python list/tuple are sliced per row (when their length matches the number of detections), matching the existing behavior for numpy.ndarray.

Changes:

  • Extend _slice_value in both sinks to slice list/tuple values when len(value) == number_of_detections.
  • Thread n = len(detections.xyxy) through parsing loops to support length-based slicing.
  • Add/extend unit tests to cover list/tuple per-row slicing across multi-append scenarios.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/supervision/detection/tools/csv_sink.py Slice list/tuple custom_data per detection row when length matches detection count.
src/supervision/detection/tools/json_sink.py Mirror the same list/tuple slicing behavior in JSON serialization.
tests/detection/test_csv.py Add coverage validating list/tuple custom_data is sliced per row (and scalar still broadcasts).
tests/detection/test_json.py Add coverage validating list/tuple custom_data is sliced per row in JSON output.

Borda and others added 10 commits April 17, 2026 21:46
…ings

- Describe broadcast vs. per-row slicing rules for custom_data values in
  CSVSink.append() and JSONSink.append()

[resolve roboflow#1] /review finding by foundry:sw-engineer + foundry:doc-scribe (report: .temp/output-review-fix-sinks-slice-list-values-2026-04-17.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
- Document n param, all four dispatch rules, Args and Returns sections

[resolve roboflow#2] /review finding by foundry:doc-scribe (report: .temp/output-review-fix-sinks-slice-list-values-2026-04-17.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
…NSink

- Document list/tuple slicing vs. broadcast semantics, Args, and Returns

[resolve roboflow#3] /review finding by foundry:doc-scribe (report: .temp/output-review-fix-sinks-slice-list-values-2026-04-17.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
- Verify CSVSink/JSONSink slice list values from detections.data per row

[resolve roboflow#4] /review finding by foundry:qa-specialist (report: .temp/output-review-fix-sinks-slice-list-values-2026-04-17.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
- Cover n=1 degenerate, mismatch fallthrough, mixed types, nested list,
  string type guard, single-element tuple

[resolve roboflow#5] /review finding by foundry:qa-specialist (report: .temp/output-review-fix-sinks-slice-list-values-2026-04-17.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
…ocstrings

[resolve roboflow#7] /review finding by foundry:doc-scribe (report: .temp/output-review-fix-sinks-slice-list-values-2026-04-17.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
---
Co-authored-by: Claude Code <noreply@anthropic.com>
@Borda Borda merged commit 06beb6f into roboflow:develop Apr 17, 2026
24 checks passed
@Borda Borda mentioned this pull request Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants