fix(dataset): stop split mutation, determinize VOC, guard collisions (#2394)

- `train_test_split` seeded the global `random` module and shuffled the caller's list in place, so `DetectionDataset.split()` reordered its own `image_paths` and polluted process-wide randomness; use a local `random.Random` and shuffle a copy
- Pascal VOC class ids were assigned in `set`-iteration and filesystem-glob order, so the same dataset produced different `class_id` values across runs; sort class names and the loaded file list
- dataset exports keyed output files on basename, silently overwriting when two entries shared a name across directories (common after `merge()`); detect basename collisions and raise
- add regression tests for split determinism, VOC id stability, and export collisions

* fix(dataset): add LabelMe collision guard, hoist pre-flight checks, make guard private
* test(dataset): add collision guard tests for as_yolo, as_pascal_voc, and boundary cases
* docs(dataset): document ValueError raises, fix stale docstrings, add non-mutation guarantee

---------

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
Jirka Borovec 2026-07-03 15:02:01 +02:00 committed by GitHub
parent 15dbbb5cb1
commit f173905c8b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 360 additions and 15 deletions

View File

@ -37,6 +37,7 @@ from supervision.dataset.formats.yolo import (
save_yolo_annotations,
)
from supervision.dataset.utils import (
_check_no_basename_collisions,
build_class_index_mapping,
map_detections_class_id,
merge_class_lists,
@ -181,7 +182,7 @@ class DetectionDataset(BaseDataset):
) -> tuple[DetectionDataset, DetectionDataset]:
"""
Splits the dataset into two parts (training and testing)
using the provided split_ratio.
using the provided split_ratio. The input dataset is not mutated.
Args:
split_ratio: The ratio of the training
@ -378,7 +379,28 @@ class DetectionDataset(BaseDataset):
polygon points to be removed from the input polygon,
in the range [0, 1). Argument is used only for segmentation datasets.
show_progress: If True, display a progress bar during saving.
Raises:
ValueError: If two image paths share the same basename (when
images_directory_path is set) or the same stem (when
annotations_directory_path is set), which would cause one
output file to overwrite another. Rename images to ensure
unique basenames before exporting a merged dataset.
"""
# Pre-flight: validate output uniqueness before writing any file
if images_directory_path:
_check_no_basename_collisions(
image_paths=self.image_paths,
key=lambda image_path: Path(image_path).name,
output_kind="image",
)
if annotations_directory_path:
_check_no_basename_collisions(
image_paths=self.image_paths,
key=lambda image_path: f"{Path(image_path).stem}.xml",
output_kind="Pascal VOC annotation",
)
if images_directory_path:
save_dataset_images(
dataset=self,
@ -386,6 +408,11 @@ class DetectionDataset(BaseDataset):
show_progress=show_progress,
)
if annotations_directory_path:
_check_no_basename_collisions(
image_paths=self.image_paths,
key=lambda image_path: f"{Path(image_path).stem}.xml",
output_kind="Pascal VOC annotation",
)
Path(annotations_directory_path).mkdir(parents=True, exist_ok=True)
for image_path, image, annotations in tqdm(
self,
@ -580,6 +607,12 @@ class DetectionDataset(BaseDataset):
`from_yolo(..., is_obb=True)`. Masks are ignored when
`is_obb=True`.
show_progress: If True, display a progress bar during saving.
Raises:
ValueError: If two image paths share the same basename (when
images_directory_path is set) or the same annotation
file name (when annotations_directory_path is set),
which would cause one output file to overwrite another.
"""
if is_obb and (
min_image_area_percentage != 0.0
@ -595,6 +628,20 @@ class DetectionDataset(BaseDataset):
UserWarning,
stacklevel=2,
)
# Pre-flight: validate output uniqueness before writing any file
if images_directory_path:
_check_no_basename_collisions(
image_paths=self.image_paths,
key=lambda image_path: Path(image_path).name,
output_kind="image",
)
if annotations_directory_path:
_check_no_basename_collisions(
image_paths=self.image_paths,
key=lambda image_path: Path(image_path).stem + ".txt",
output_kind="YOLO annotation",
)
if images_directory_path is not None:
save_dataset_images(
dataset=self,

View File

@ -7,6 +7,7 @@ from typing import TYPE_CHECKING, Any
import numpy as np
import numpy.typing as npt
from supervision.dataset.utils import _check_no_basename_collisions
from supervision.detection.core import Detections
from supervision.detection.utils.converters import (
mask_to_polygons,
@ -363,6 +364,10 @@ def save_labelme_annotations(
annotations_directory_path: Directory where the LabelMe ``.json`` files
are written (created if it does not exist).
Raises:
ValueError: If two image paths map to the same output .json stem,
which would cause one annotation file to overwrite another.
Examples:
```python
import supervision as sv
@ -375,6 +380,11 @@ def save_labelme_annotations(
)
```
"""
_check_no_basename_collisions(
image_paths=dataset.image_paths,
key=lambda image_path: f"{Path(image_path).stem}.json",
output_kind="LabelMe annotation",
)
Path(annotations_directory_path).mkdir(parents=True, exist_ok=True)
for image_path, image, detections in dataset:
image_height, image_width, _ = image.shape

View File

@ -184,8 +184,7 @@ def load_pascal_voc_annotations(
show_progress: bool = False,
) -> tuple[list[str], list[str], dict[str, Detections]]:
"""
Loads PASCAL VOC XML annotations and returns the image name,
a Detections instance, and a list of class names.
Load Pascal VOC XML annotations in sorted image-path order.
Args:
images_directory_path: The path to the directory containing the images.
@ -196,17 +195,17 @@ def load_pascal_voc_annotations(
show_progress: If True, display a progress bar during loading.
Returns:
A tuple with a list
of class names, a list of paths to images, and a dictionary with image
paths as keys and corresponding Detections instances as values.
A tuple with a list of class names, a sorted list of paths to images,
and a dictionary with image paths as keys and corresponding
Detections instances as values.
"""
image_paths = [
image_paths = sorted(
str(path)
for path in list_files_with_extensions(
directory=images_directory_path, extensions=["jpg", "jpeg", "png"]
)
]
)
classes: list[str] = []
annotations = {}
@ -270,6 +269,14 @@ def detections_from_xml_obj(
</object>
</annotation>
Args:
root: Parsed Pascal VOC ``<annotation>`` XML element.
classes: Existing class names used to assign stable class ids.
resolution_wh: Image resolution as ``(width, height)`` for mask
rasterization.
force_masks: If True, returns a mask array for every object even when
no ``<polygon>`` element is present.
Returns:
A tuple containing a Detections object and an
updated list of class names, extended with the class names
@ -322,7 +329,7 @@ def detections_from_xml_obj(
# https://github.com/roboflow/supervision/issues/144
xyxy_arr -= 1
for k in set(class_names):
for k in sorted(set(class_names)):
if k not in extended_classes:
extended_classes.append(k)
class_id = np.array(

View File

@ -12,7 +12,10 @@ from PIL import Image
from tqdm.auto import tqdm
from supervision.config import ORIENTED_BOX_COORDINATES
from supervision.dataset.utils import approximate_mask_with_polygons
from supervision.dataset.utils import (
_check_no_basename_collisions,
approximate_mask_with_polygons,
)
from supervision.detection.core import Detections
from supervision.detection.utils._typing import _DetectionDataType
from supervision.detection.utils.converters import polygon_to_mask, polygon_to_xyxy
@ -470,6 +473,11 @@ def save_yolo_annotations(
>>> dataset = DetectionDataset(classes=["cat"], images={}, annotations={})
>>> save_yolo_annotations(dataset, "/tmp/labels")
"""
_check_no_basename_collisions(
image_paths=dataset.image_paths,
key=lambda image_path: _image_name_to_annotation_name(Path(image_path).name),
output_kind="YOLO annotation",
)
Path(annotations_directory_path).mkdir(parents=True, exist_ok=True)
for image_path, image, annotation in tqdm(
dataset,

View File

@ -4,6 +4,7 @@ import copy
import os
import random
import shutil
from collections.abc import Callable
from pathlib import Path
from typing import TYPE_CHECKING, TypeVar, cast
@ -126,6 +127,51 @@ def map_detections_class_id(
return detections_copy
def _check_no_basename_collisions(
image_paths: list[str],
key: Callable[[str], str],
output_kind: str,
) -> None:
"""Raise if two image paths would be written to the same output file.
Dataset image paths may share a basename when they originate from different
directories (a legal, common state after :meth:`DetectionDataset.merge`).
Exporting them into a single flat output directory keyed on the basename or
stem would silently overwrite one file with another and mispair images with
their annotations. This guard detects such collisions before any file is
written and names the colliding source paths.
Args:
image_paths: The dataset image paths about to be written.
key: Maps an image path to the output file name it would be written to.
output_kind: Human-readable description of the output (e.g. ``"image"``
or ``"YOLO annotation"``) used in the error message.
Raises:
ValueError: If two image paths map to the same output file name.
Examples:
>>> from pathlib import Path
>>> from supervision.dataset.utils import _check_no_basename_collisions
>>> _check_no_basename_collisions(
... ["a/img.jpg", "b/img.jpg"], lambda p: Path(p).name, "image"
... )
Traceback (most recent call last):
...
ValueError: Cannot export dataset: image paths 'a/img.jpg' and ...
"""
seen: dict[str, str] = {}
for image_path in image_paths:
output_name = key(image_path)
if output_name in seen:
raise ValueError(
f"Cannot export dataset: image paths {seen[output_name]!r} and "
f"{image_path!r} both map to {output_kind} file {output_name!r}. "
"Ensure all image basenames are unique before exporting."
)
seen[output_name] = image_path
def save_dataset_images(
dataset: DetectionDataset,
images_directory_path: str,
@ -149,6 +195,11 @@ def save_dataset_images(
>>> dataset = DetectionDataset(classes=["cat"], images={}, annotations={})
>>> save_dataset_images(dataset, "/tmp/images")
"""
_check_no_basename_collisions(
image_paths=dataset.image_paths,
key=lambda image_path: Path(image_path).name,
output_kind="image",
)
Path(images_directory_path).mkdir(parents=True, exist_ok=True)
for image_path in tqdm(
dataset.image_paths,
@ -179,13 +230,21 @@ def train_test_split(
shuffle: Whether to shuffle the data before splitting.
Returns:
The split data.
"""
if random_state is not None:
random.seed(random_state)
The split data. The input list is copied and never mutated.
Examples:
>>> train, test = train_test_split(
... [1, 2, 3, 4, 5], train_ratio=0.6, random_state=0
... )
>>> len(train), len(test)
(3, 2)
"""
rng = random.Random(random_state) # noqa: S311 — dataset split, not cryptographic
if shuffle:
random.shuffle(data)
data = list(data)
rng.shuffle(data)
else:
data = list(data) # copy to guarantee non-mutation
split_index = int(len(data) * train_ratio)
return data[:split_index], data[split_index:]

View File

@ -1,5 +1,7 @@
from contextlib import ExitStack as DoesNotRaise
from pathlib import Path
import cv2
import numpy as np
import pytest
from defusedxml import ElementTree
@ -7,6 +9,7 @@ from defusedxml import ElementTree
from supervision.dataset.formats.pascal_voc import (
detections_from_xml_obj,
detections_to_pascal_voc,
load_pascal_voc_annotations,
object_to_pascal_voc,
parse_polygon_points,
)
@ -242,3 +245,58 @@ def test_detections_from_xml_obj_mixed_polygon_and_bbox_masks_aligned(
assert detections.mask.shape == (2, 30, 30)
assert detections.mask[0].any()
assert not detections.mask[1].any()
def _write_voc_sample(
images_dir: Path, annotations_dir: Path, stem: str, class_names: list[str]
) -> None:
"""Write one VOC image plus its bbox-only XML annotation to disk."""
cv2.imwrite(str(images_dir / f"{stem}.png"), np.zeros((20, 20, 3), dtype=np.uint8))
objects = "".join(
f"<object><name>{name}</name><bndbox><xmin>1</xmin><ymin>1</ymin>"
f"<xmax>10</xmax><ymax>10</ymax></bndbox></object>"
for name in class_names
)
(annotations_dir / f"{stem}.xml").write_text(f"<annotation>{objects}</annotation>")
class TestLoadPascalVocDeterministicClasses:
"""Regression tests for deterministic VOC class ordering (DAT-03)."""
def test_classes_sorted_within_file(self, tmp_path: Path) -> None:
"""Class names from one file are assigned ids in sorted, stable order."""
images_dir = tmp_path / "images"
images_dir.mkdir()
annotations_dir = tmp_path / "annotations"
annotations_dir.mkdir()
_write_voc_sample(images_dir, annotations_dir, "img", ["zebra", "ant", "mango"])
classes, _, _ = load_pascal_voc_annotations(
images_directory_path=str(images_dir),
annotations_directory_path=str(annotations_dir),
)
assert classes == sorted(classes) == ["ant", "mango", "zebra"]
def test_repeated_loads_give_identical_class_ids(self, tmp_path: Path) -> None:
"""Two loads of the same multi-file VOC set produce identical class ids."""
images_dir = tmp_path / "images"
images_dir.mkdir()
annotations_dir = tmp_path / "annotations"
annotations_dir.mkdir()
_write_voc_sample(images_dir, annotations_dir, "a_img", ["zebra"])
_write_voc_sample(images_dir, annotations_dir, "b_img", ["ant", "mango"])
first = load_pascal_voc_annotations(
images_directory_path=str(images_dir),
annotations_directory_path=str(annotations_dir),
)
second = load_pascal_voc_annotations(
images_directory_path=str(images_dir),
annotations_directory_path=str(annotations_dir),
)
assert first[0] == second[0]
assert {p: d.class_id.tolist() for p, d in first[2].items()} == {
p: d.class_id.tolist() for p, d in second[2].items()
}

View File

@ -383,3 +383,88 @@ class TestDetectionDatasetInMemoryImages:
ds_b = self._build_dataset({"img1.jpg": _create_image(fill_value=2)})
assert ds_a != ds_b
class TestDetectionDatasetExportCollisions:
"""Regression tests for the basename-collision guard on export (DAT-04)."""
def test_as_yolo_raises_on_same_basename_images(self, tmp_path: Path) -> None:
"""Same-basename images from different directories must not overwrite."""
dataset = DetectionDataset(
classes=["cat"],
images=["dir_a/img.png", "dir_b/img.png"],
annotations={
"dir_a/img.png": _create_detections(
xyxy=[[0, 0, 10, 10]], class_id=[0]
),
"dir_b/img.png": _create_detections(
xyxy=[[0, 0, 10, 10]], class_id=[0]
),
},
)
with pytest.raises(ValueError, match="both map to image file"):
dataset.as_yolo(images_directory_path=str(tmp_path / "images"))
def test_as_yolo_raises_on_same_stem_annotations(self, tmp_path: Path) -> None:
"""Same-stem images must not overwrite annotations."""
dataset = DetectionDataset(
classes=["cat"],
images=["dir_a/img.jpg", "dir_b/img.png"],
annotations={
"dir_a/img.jpg": _create_detections(
xyxy=[[0, 0, 10, 10]], class_id=[0]
),
"dir_b/img.png": _create_detections(
xyxy=[[0, 0, 10, 10]], class_id=[0]
),
},
)
with pytest.raises(ValueError, match="both map to YOLO annotation file"):
dataset.as_yolo(
images_directory_path=str(tmp_path / "images"),
annotations_directory_path=str(tmp_path / "labels"),
)
def test_as_pascal_voc_raises_on_same_basename_images(self, tmp_path: Path) -> None:
"""Same-basename images must not overwrite image files on export."""
dataset = DetectionDataset(
classes=["cat"],
images=["dir_a/img.jpg", "dir_b/img.jpg"],
annotations={
"dir_a/img.jpg": _create_detections(
xyxy=[[0, 0, 10, 10]], class_id=[0]
),
"dir_b/img.jpg": _create_detections(
xyxy=[[0, 0, 10, 10]], class_id=[0]
),
},
)
with pytest.raises(ValueError, match="both map to image file"):
dataset.as_pascal_voc(
images_directory_path=str(tmp_path / "images"),
)
def test_as_pascal_voc_raises_on_same_stem_annotations(
self, tmp_path: Path
) -> None:
"""Same-stem images must not overwrite annotations."""
dataset = DetectionDataset(
classes=["cat"],
images=["dir_a/img.jpg", "dir_b/img.png"],
annotations={
"dir_a/img.jpg": _create_detections(
xyxy=[[0, 0, 10, 10]], class_id=[0]
),
"dir_b/img.png": _create_detections(
xyxy=[[0, 0, 10, 10]], class_id=[0]
),
},
)
with pytest.raises(ValueError, match="both map to Pascal VOC annotation file"):
dataset.as_pascal_voc(
annotations_directory_path=str(tmp_path / "annotations"),
)

View File

@ -1,10 +1,13 @@
import random
from contextlib import ExitStack as DoesNotRaise
from pathlib import Path
from typing import TypeVar
import pytest
from supervision import Detections
from supervision.dataset.utils import (
_check_no_basename_collisions,
build_class_index_mapping,
map_detections_class_id,
merge_class_lists,
@ -229,3 +232,71 @@ def test_map_detections_class_id(
source_to_target_mapping=source_to_target_mapping, detections=detections
)
assert result == expected_result
class TestTrainTestSplitRngIsolation:
"""Regression tests for train_test_split RNG isolation (DAT-02)."""
def test_does_not_mutate_input_list(self) -> None:
"""split() must not reorder the caller's list in place."""
data = list(range(10))
original = data.copy()
train_test_split(data=data, train_ratio=0.5, random_state=42, shuffle=True)
assert data == original
def test_does_not_pollute_global_rng(self) -> None:
"""split() must not disturb the process-global random state."""
state_before = random.getstate()
train_test_split(
data=list(range(10)), train_ratio=0.5, random_state=42, shuffle=True
)
assert random.getstate() == state_before
def test_result_independent_of_global_rng(self) -> None:
"""A fixed random_state yields the same split regardless of global RNG."""
first = train_test_split(
data=list(range(10)), train_ratio=0.5, random_state=42, shuffle=True
)
for _ in range(5):
random.random() # noqa: S311 — perturb global RNG; split must ignore it
second = train_test_split(
data=list(range(10)), train_ratio=0.5, random_state=42, shuffle=True
)
assert first == second
class TestCheckNoBasenameCollisions:
"""Regression tests for export basename collision detection (DAT-04)."""
def test_empty_list_does_not_raise(self) -> None:
"""Empty image_paths must not raise."""
_check_no_basename_collisions(
image_paths=[],
key=lambda image_path: Path(image_path).name,
output_kind="image",
)
def test_single_path_does_not_raise(self) -> None:
"""Single image path cannot collide; must not raise."""
_check_no_basename_collisions(
image_paths=["a/img.jpg"],
key=lambda image_path: Path(image_path).name,
output_kind="image",
)
def test_raises_on_colliding_output_names(self) -> None:
"""Two source paths mapping to one output name must raise ValueError."""
with pytest.raises(ValueError, match="both map to image file"):
_check_no_basename_collisions(
image_paths=["a/img.jpg", "b/img.jpg"],
key=lambda image_path: Path(image_path).name,
output_kind="image",
)
def test_passes_on_unique_output_names(self) -> None:
"""Distinct output names must not raise."""
_check_no_basename_collisions(
image_paths=["a/img1.jpg", "b/img2.jpg"],
key=lambda image_path: Path(image_path).name,
output_kind="image",
)