fix(datasets): require pipeline options instead of substituting wrong defaults - #1413
Open
Maryyyyyyyam142 wants to merge 1 commit into
Open
Conversation
… defaults The dataset builders read the same image-pipeline options from `args`, but `build_roboflow_from_coco`, `build_roboflow_from_yolo` and `build_o365_raw` read them via `getattr` with literal fallbacks that contradict the real config defaults, while `build_coco` reads most of them directly: square_resize_div_64 fallback False, TrainConfig default True segmentation_head fallback False, True on every seg variant multi_scale fallback False, TrainConfig default True expanded_scales fallback False, TrainConfig default True patch_size fallback 16, variant-dependent (12/14/16) num_windows fallback 4, 2 on every released variant `patch_size` and `num_windows` have no default on `ModelConfig`, so no constant is correct; the pair the builders chose, (16, 4), matches no shipped variant. A caller passing an incomplete namespace — supported and documented usage, since the builders are re-exported from `rfdetr.datasets` — silently trained a different pipeline: multi-scale off, and a scale set of [192..832] instead of [352..672] at resolution 512. Read these seven options directly in all four builders so an incomplete namespace fails loudly. The remaining `getattr` fallbacks are left alone: their values match the config default, or the absence is meaningful (keypoint fields absent means detection-only, per the comment at coco.py). This is latent on the `RFDETRDataModule` path, where `_namespace_from_configs` populates every field.
Maryyyyyyyam142
requested review from
Borda,
SkalskiP,
isaacrob and
probicheaux
as code owners
August 31, 2026 12:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1412
The four dataset builders derive the same image-pipeline options from
args, but disagree on how.build_cocoreads most of them directly;build_roboflow_from_coco,build_roboflow_from_yoloandbuild_o365_rawusegetattrwith literal fallbacks — five of which contradict the real config default:square_resize_div_64FalseTrue(TrainConfig)segmentation_headFalseTrueon every seg variantmulti_scaleFalseTrue(TrainConfig)expanded_scalesFalseTrue(TrainConfig)patch_size1612seg/keypoint,14basenum_windows42on every released variantpatch_sizeandnum_windowshave no default onModelConfig, so no constant is correct — and(16, 4)matches no shipped variant.Change
All four builders now read these seven options directly, so an incomplete namespace raises
AttributeErrorinstead of silently building a different pipeline.do_random_resize_via_paddingis included for cluster parity; its fallback already matched the default, so that one is a no-op.The remaining
getattrfallbacks are deliberately untouched —aug_config,scale_jitterandaugmentation_backendmatch their config default, and the keypoint fields' absence is meaningful ("detection-only", per the existing comment atcoco.py:1301).Scope
Latent on the
RFDETRDataModulepath:_namespace_from_configspopulates every field, so normalmodel.train(...)is unaffected. The exposure is the direct-call path, which the builders' re-export fromrfdetr.datasetsand the comment atcoco.py:1318-1322both document as supported.Tests
New
tests/datasets/test_builder_options.pycovers the cross-builder contract:develop)num_windows=2not4,multi_scale=True, seg variants atpatch_size=12withinclude_masks=TrueFour tests in
test_coco.pypassed partial namespaces and relied on the old fallbacks. They now use a_pipeline_argshelper that spells out the values those fallbacks used to produce, so their behavior is unchanged and what they depend on is visible.674 passedacrosstests/datasets/andsrc/rfdetr/datasets/.ruff,ruff format,docformatterandcodespellpass on the changed files;mypyreports nothing in them.