Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions src/rfdetr/datasets/coco.py
Original file line number Diff line number Diff line change
Expand Up @@ -1309,8 +1309,10 @@ def build_coco(image_set: str, args: Any, resolution: int) -> CocoDetection:

img_folder, ann_file = PATHS[image_set.split("_", maxsplit=1)[0]]

square_resize_div_64 = getattr(args, "square_resize_div_64", False)
include_masks = getattr(args, "segmentation_head", False)
# Pipeline options are read directly, never via getattr with a literal default;
# see build_roboflow_from_coco in rfdetr/datasets/coco.py for why.
square_resize_div_64 = args.square_resize_div_64
include_masks = args.segmentation_head
include_keypoints = has_keypoints
num_keypoints_per_class = getattr(args, "num_keypoints_per_class", [])
aug_config = getattr(args, "aug_config", None)
Expand Down Expand Up @@ -1419,13 +1421,18 @@ def build_roboflow_from_coco(image_set: str, args: Any, resolution: int) -> Coco

split = image_set.split("_", maxsplit=1)[0]
img_folder, ann_file = PATHS[split]
square_resize_div_64 = getattr(args, "square_resize_div_64", False)
include_masks = getattr(args, "segmentation_head", False)
multi_scale = getattr(args, "multi_scale", False)
expanded_scales = getattr(args, "expanded_scales", False)
do_random_resize_via_padding = getattr(args, "do_random_resize_via_padding", False)
patch_size = getattr(args, "patch_size", 16)
num_windows = getattr(args, "num_windows", 4)
# Read directly rather than via getattr with a literal default: these options have no safe constant.
# patch_size, num_windows and segmentation_head are variant-dependent, and square_resize_div_64,
# multi_scale and expanded_scales all default to True on TrainConfig. An incomplete namespace must fail
# here instead of silently building a different pipeline (GitHub #N). The optional fields below keep getattr on
# purpose: their absence means "detection, no custom augmentation", which is a real and safe default.
square_resize_div_64 = args.square_resize_div_64
include_masks = args.segmentation_head
multi_scale = args.multi_scale
expanded_scales = args.expanded_scales
do_random_resize_via_padding = args.do_random_resize_via_padding
patch_size = args.patch_size
num_windows = args.num_windows
# Roboflow detection exports omit keypoint schema/flip-pair fields; missing values mean detection-only.
include_keypoints = getattr(args, "use_grouppose_keypoints", False)
num_keypoints_per_class = getattr(args, "num_keypoints_per_class", [])
Expand Down
4 changes: 3 additions & 1 deletion src/rfdetr/datasets/o365.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ def build_o365_raw(image_set: str, args: Any, resolution: int) -> CocoDetection:

from rfdetr.datasets.kornia_transforms import is_gpu_postprocess, resolve_backend_for_build

square_resize_div_64 = getattr(args, "square_resize_div_64", False)
# Pipeline options are read directly, never via getattr with a literal default;
# see build_roboflow_from_coco in rfdetr/datasets/coco.py for why.
square_resize_div_64 = args.square_resize_div_64
scale_jitter = getattr(args, "scale_jitter", True)
augmentation_backend = getattr(args, "augmentation_backend", "cpu")
resolved_backend = resolve_backend_for_build(augmentation_backend)
Expand Down
16 changes: 9 additions & 7 deletions src/rfdetr/datasets/yolo.py
Original file line number Diff line number Diff line change
Expand Up @@ -1051,13 +1051,15 @@ def build_roboflow_from_yolo(image_set: str, args: Any, resolution: int) -> Yolo
img_folder, lb_folder = _resolve_yolo_split_dirs(root, data_file, split_key)
if split_key == "test":
_validate_yolo_test_split(img_folder, lb_folder)
square_resize_div_64 = getattr(args, "square_resize_div_64", False)
include_masks = getattr(args, "segmentation_head", False)
multi_scale = getattr(args, "multi_scale", False)
expanded_scales = getattr(args, "expanded_scales", False)
do_random_resize_via_padding = getattr(args, "do_random_resize_via_padding", False)
patch_size = getattr(args, "patch_size", 16)
num_windows = getattr(args, "num_windows", 4)
# Pipeline options are read directly, never via getattr with a literal default;
# see build_roboflow_from_coco in rfdetr/datasets/coco.py for why.
square_resize_div_64 = args.square_resize_div_64
include_masks = args.segmentation_head
multi_scale = args.multi_scale
expanded_scales = args.expanded_scales
do_random_resize_via_padding = args.do_random_resize_via_padding
patch_size = args.patch_size
num_windows = args.num_windows
aug_config = getattr(args, "aug_config", None)
scale_jitter = getattr(args, "scale_jitter", True)
include_keypoints = getattr(args, "use_grouppose_keypoints", False)
Expand Down
203 changes: 203 additions & 0 deletions tests/datasets/test_builder_options.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
# ------------------------------------------------------------------------
# RF-DETR
# Copyright (c) 2025 Roboflow. All Rights Reserved.
# Licensed under the Apache License, Version 2.0 [see LICENSE for details]
# ------------------------------------------------------------------------
"""Cross-builder contract for the image-pipeline options every dataset builder reads from ``args``.

``build_coco``, ``build_roboflow_from_coco``, ``build_roboflow_from_yolo`` and ``build_o365_raw`` all derive the same
resize/geometry options from the namespace the DataModule hands them. These options have no safe constant fallback:
``patch_size``, ``num_windows`` and ``segmentation_head`` are variant-dependent, and ``square_resize_div_64``,
``multi_scale`` and ``expanded_scales`` default to ``True`` on ``TrainConfig``. Substituting a literal for a missing
attribute silently trains a different pipeline, so every builder must read them directly and fail loudly instead.
"""

import types
from typing import Any
from unittest.mock import MagicMock, patch

import pytest

from rfdetr._namespace import _namespace_from_configs
from rfdetr.config import ModelConfig, RFDETRSegSmallConfig, RFDETRSmallConfig, TrainConfig
from rfdetr.datasets.coco import build_coco, build_roboflow_from_coco
from rfdetr.datasets.o365 import build_o365_raw
from rfdetr.datasets.yolo import build_roboflow_from_yolo

# The options that must come from args, with no literal fallback. Ordered as the builders read them.
REQUIRED_PIPELINE_OPTIONS = (
"square_resize_div_64",
"segmentation_head",
"multi_scale",
"expanded_scales",
"do_random_resize_via_padding",
"patch_size",
"num_windows",
)


def training_namespace(model_config: ModelConfig, dataset_dir: str = "/fake/dataset") -> types.SimpleNamespace:
"""Build the merged model/train namespace that ``RFDETRDataModule`` hands to a dataset builder.

Uses the real :func:`~rfdetr._namespace._namespace_from_configs` rather than a hand-rolled stub so the
test asserts against the values a builder actually receives during training.

Args:
model_config: Architecture config supplying ``patch_size``, ``num_windows`` and ``segmentation_head``.
dataset_dir: Dataset root recorded on the namespace.

Returns:
Namespace carrying every field the dataset builders read.

Examples:
>>> namespace = training_namespace(RFDETRSmallConfig())
>>> (namespace.multi_scale, namespace.num_windows, namespace.square_resize_div_64)
(True, 2, True)
"""
return _namespace_from_configs(model_config, TrainConfig(dataset_dir=dataset_dir))


def build_roboflow_coco_with_mocks(args: Any, resolution: int = 512) -> dict[str, Any]:
"""Run ``build_roboflow_from_coco`` with the filesystem and transform builders mocked out.

Args:
args: Namespace forwarded to the builder.
resolution: Base square resolution.

Returns:
``{"square_resize_used": bool, "transform_kwargs": dict, "dataset_kwargs": dict}`` describing which
transform branch ran, the kwargs it received, and the kwargs ``CocoDetection`` received.

Examples:
>>> captured = build_roboflow_coco_with_mocks(training_namespace(RFDETRSmallConfig()))
>>> captured["transform_kwargs"]["multi_scale"]
True
"""
with (
patch("rfdetr.datasets.coco.Path") as mock_path,
patch("rfdetr.datasets.coco.logger"), # the builder logs to stdout, which would break the doctest
patch("rfdetr.datasets.coco.make_coco_transforms_square_div_64") as mock_square,
patch("rfdetr.datasets.coco.make_coco_transforms") as mock_plain,
patch("rfdetr.datasets.coco.CocoDetection", return_value=MagicMock()) as mock_dataset,
):
mock_path.return_value.exists.return_value = True
mock_square.return_value = mock_plain.return_value = MagicMock()
build_roboflow_from_coco("train", args, resolution=resolution)

used = mock_square if mock_square.called else mock_plain
return {
"square_resize_used": mock_square.called,
"transform_kwargs": used.call_args.kwargs,
"dataset_kwargs": mock_dataset.call_args.kwargs,
}


def call_yolo_builder(args: Any, resolution: int = 512) -> None:
"""Run ``build_roboflow_from_yolo`` with the filesystem and split resolution mocked out.

Args:
args: Namespace forwarded to the builder.
resolution: Base square resolution.

Examples:
>>> call_yolo_builder(training_namespace(RFDETRSmallConfig()))
"""
fake_dirs = (MagicMock(), MagicMock())
with (
patch("rfdetr.datasets.yolo.Path") as mock_path,
patch("rfdetr.datasets.yolo._resolve_yolo_split_dirs", return_value=fake_dirs),
patch("rfdetr.datasets.yolo.make_coco_transforms_square_div_64", return_value=MagicMock()),
patch("rfdetr.datasets.yolo.make_coco_transforms", return_value=MagicMock()),
patch("rfdetr.datasets.yolo.YoloDetection", return_value=MagicMock()),
):
mock_path.return_value.exists.return_value = True
build_roboflow_from_yolo("train", args, resolution=resolution)


def call_o365_builder(args: Any, resolution: int = 512) -> None:
"""Run ``build_o365_raw`` with the dataset class and transform builders mocked out.

Args:
args: Namespace forwarded to the builder.
resolution: Base square resolution.

Examples:
>>> call_o365_builder(training_namespace(RFDETRSmallConfig()))
"""
with (
patch("rfdetr.datasets.o365.CocoDetection", return_value=MagicMock()),
patch("rfdetr.datasets.o365.make_coco_transforms_square_div_64", return_value=MagicMock()),
patch("rfdetr.datasets.o365.make_coco_transforms", return_value=MagicMock()),
):
build_o365_raw("train", args, resolution=resolution)


class TestPipelineOptionsHaveNoSilentFallback:
"""A namespace missing a pipeline option must raise, not train a silently different pipeline."""

@pytest.fixture
def partial_namespace(self, tmp_path) -> types.SimpleNamespace:
"""Namespace carrying only the fields unrelated to the image pipeline."""
return types.SimpleNamespace(
dataset_dir=str(tmp_path),
coco_path=str(tmp_path),
augmentation_backend="cpu",
)

def test_roboflow_coco_builder_raises(self, partial_namespace: types.SimpleNamespace) -> None:
"""build_roboflow_from_coco must not substitute a literal for a missing pipeline option."""
with pytest.raises(AttributeError, match="square_resize_div_64"):
build_roboflow_coco_with_mocks(partial_namespace)

def test_roboflow_yolo_builder_raises(self, partial_namespace: types.SimpleNamespace) -> None:
"""build_roboflow_from_yolo must not substitute a literal for a missing pipeline option."""
with pytest.raises(AttributeError, match="square_resize_div_64"):
call_yolo_builder(partial_namespace)

def test_o365_builder_raises(self, partial_namespace: types.SimpleNamespace) -> None:
"""build_o365_raw must not substitute a literal for a missing pipeline option."""
with pytest.raises(AttributeError, match="square_resize_div_64"):
call_o365_builder(partial_namespace)

def test_coco_builder_raises(self, partial_namespace: types.SimpleNamespace) -> None:
"""build_coco must not substitute a literal for a missing pipeline option."""
with (
patch("rfdetr.datasets.coco.Path") as mock_path,
patch("rfdetr.datasets.coco.CocoDetection", return_value=MagicMock()),
):
mock_path.return_value.exists.return_value = True
with pytest.raises(AttributeError, match="square_resize_div_64"):
build_coco("train", partial_namespace, resolution=512)


class TestConfigValuesReachTheTransformPipeline:
"""The values the builders forward must be the configured ones, not the old literal fallbacks."""

@pytest.mark.parametrize(
"option,expected",
[
pytest.param("multi_scale", True, id="multi_scale_stays_enabled"),
pytest.param("expanded_scales", True, id="expanded_scales_stays_enabled"),
pytest.param("num_windows", 2, id="num_windows_from_variant_not_4"),
pytest.param("patch_size", 16, id="patch_size_from_variant"),
],
)
def test_small_variant_option_is_forwarded(self, option: str, expected: Any) -> None:
"""RFDETRSmall's real option values must reach the transform builder unchanged."""
captured = build_roboflow_coco_with_mocks(training_namespace(RFDETRSmallConfig()))
assert captured["transform_kwargs"][option] == expected

def test_square_resize_default_selects_the_square_branch(self) -> None:
"""TrainConfig.square_resize_div_64 defaults to True, so the square-resize builder must run."""
captured = build_roboflow_coco_with_mocks(training_namespace(RFDETRSmallConfig()))
assert captured["square_resize_used"] is True

def test_segmentation_variant_enables_masks(self) -> None:
"""A segmentation variant must reach CocoDetection with include_masks=True."""
captured = build_roboflow_coco_with_mocks(training_namespace(RFDETRSegSmallConfig()))
assert captured["dataset_kwargs"]["include_masks"] is True

def test_seg_variant_patch_size_is_not_the_old_literal(self) -> None:
"""Segmentation variants use patch_size=12; the removed fallback would have forced 16."""
captured = build_roboflow_coco_with_mocks(training_namespace(RFDETRSegSmallConfig()))
assert captured["transform_kwargs"]["patch_size"] == 12
41 changes: 37 additions & 4 deletions tests/datasets/test_coco.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,39 @@ def _write_roboflow_keypoint_coco(path: Path, *, category_id: int = 0) -> None:
path.write_text(json.dumps(data), encoding="utf-8")


def _pipeline_args(dataset_dir: object, **overrides: object) -> types.SimpleNamespace:
"""Build a builder namespace carrying the image-pipeline options the dataset builders require.

The builders read these options directly, with no literal fallback, so a namespace handed to one must spell
them out. The values here reproduce the pipeline the removed ``getattr`` fallbacks used to produce, which
keeps tests that only care about label space or backend resolution behaviourally unchanged. Tests asserting
that the *configured* values reach the pipeline live in ``tests/datasets/test_builder_options.py``.

Args:
dataset_dir: Dataset root recorded on the namespace.
**overrides: Extra fields to add, or pipeline options to replace.

Returns:
Namespace accepted by the Roboflow COCO and YOLO builders.

Examples:
>>> _pipeline_args("/tmp/ds", augmentation_backend="gpu").multi_scale
False
"""
options = {
"dataset_dir": str(dataset_dir),
"square_resize_div_64": False,
"segmentation_head": False,
"multi_scale": False,
"expanded_scales": False,
"do_random_resize_via_padding": False,
"patch_size": 16,
"num_windows": 4,
}
options.update(overrides)
return types.SimpleNamespace(**options)


class TestLoadClassesHierarchy:
"""Regression tests for ``_load_classes`` supercategory filtering (#609).

Expand Down Expand Up @@ -629,7 +662,7 @@ def test_gpu_backend_no_cuda_raises_runtime_error(self, tmp_path: Path) -> None:

from rfdetr.datasets.coco import build_roboflow_from_coco

args = types.SimpleNamespace(dataset_dir=str(tmp_path), augmentation_backend="gpu")
args = _pipeline_args(tmp_path, augmentation_backend="gpu")
with (
patch("rfdetr.datasets.kornia_transforms._has_cuda_device", return_value=False),
pytest.raises(RuntimeError, match="CUDA"),
Expand All @@ -643,7 +676,7 @@ def test_gpu_backend_no_kornia_raises_import_error(self, tmp_path: Path) -> None
from rfdetr.config import AugmentationBackend
from rfdetr.datasets.coco import build_roboflow_from_coco

args = types.SimpleNamespace(dataset_dir=str(tmp_path), augmentation_backend="gpu")
args = _pipeline_args(tmp_path, augmentation_backend="gpu")
with (
patch("rfdetr.datasets.kornia_transforms._has_cuda_device", return_value=True),
patch.object(AugmentationBackend, "_is_available", lambda self: self is not AugmentationBackend.KORNIA),
Expand Down Expand Up @@ -1768,7 +1801,7 @@ def test_val_split_reuses_the_train_label_mapping(self, tmp_path: Path) -> None:
"""A grouping category annotated in train only keeps its train label slot in val instead of shifting it."""
_write_roboflow_hierarchy_split(tmp_path / "train", [0, 1])
_write_roboflow_hierarchy_split(tmp_path / "valid", [1])
args = types.SimpleNamespace(dataset_dir=str(tmp_path))
args = _pipeline_args(tmp_path)

train_dataset = build_roboflow_from_coco("train", args, resolution=64)
val_dataset = build_roboflow_from_coco("val", args, resolution=64)
Expand All @@ -1779,7 +1812,7 @@ def test_val_targets_use_train_label_indices(self, tmp_path: Path) -> None:
"""Val targets carry the label index training assigned, not the one val's own coverage would produce."""
_write_roboflow_hierarchy_split(tmp_path / "train", [0, 1])
_write_roboflow_hierarchy_split(tmp_path / "valid", [1])
args = types.SimpleNamespace(dataset_dir=str(tmp_path))
args = _pipeline_args(tmp_path)

val_dataset = build_roboflow_from_coco("val", args, resolution=64)
_, target = val_dataset[0]
Expand Down