Move some TestImageDatasetExport to TestDatasetExport - #1633
Conversation
Move the format-contract tests (COCO/YOLO/Pascal/captions output correctness, category sorting, mask handling, path defaults) from TestImageDatasetExport into a new TestDatasetExport, driving DatasetExport directly with the image mapping as substrate. These assert behavior shared by every sample type, so they belong to the base. Slim test_image_dataset_export.py to the image-specific delta: unit tests for the image_sample_to_image mapping (absolute path for COCO, relative name for YOLO/Pascal) plus one smoke that ImageDataset.export() binds it. No behavior change; coverage preserved. An upcoming VideoFrameDatasetExport only needs its own small mapping tests, not a copy of the format suite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR reorganizes image dataset export tests and adds dedicated coverage for COCO captions, COCO detections and segmentation masks, YOLO detections, and Pascal VOC masks. Image-specific tests now focus on query forwarding and absolute or relative filename mapping. ChangesDataset export test coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e04f038de8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Split the DatasetExport format tests into per-format files (test_dataset_export__coco_object_detections.py, __yolo_object_detections.py, __coco_captions.py, __coco_segmentation_masks.py, __pascal_voc.py), matching the test_<subject>__<topic>.py convention used in tests/core/image. The shared DatasetExport factory moves to tests/export/helpers.py. Restore query-forwarding coverage in the ImageDataset.export() smoke test, and drop the "bind"/"substrate" wording from docstrings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
build_dataset_export only re-did what ImageDatasetExport already does (bind the image sample-to-image mapping), so instantiate ImageDatasetExport directly in the format tests and remove tests/export/helpers.py. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
/review |
What has changed and why
Move tests from TestImageDatasetExport to the new class TestDatasetExport. Split the latter across different files to keep the files small.
TestImageDatasetExport now only tests sample_to_image and that it is used correctly.
Review by Codex 5.5 extra high