keep original rotation on import-export in yolo oriented boxes #72
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces changes to the YOLO format plugin in Datumaro, focusing on bounding box and polygon annotation handling. The modifications include making a function public in the converter, adding a rotation synchronization method in the extractor, and updating the dataset comparison logic in the test suite. These changes aim to improve the handling of oriented bounding boxes and their transformations across different stages of annotation processing. Changes
Sequence DiagramsequenceDiagram
participant Converter
participant Extractor
participant Annotation
Converter->>Annotation: bbox_annotation_as_polygon()
Extractor->>Annotation: _rotate_bbox_to_sync_exported_annotation_with_imported()
Annotation-->>Extractor: Adjusted Bounding Box
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/datumaro/plugins/yolo_format/extractor.py (1)
505-548: Consider documenting assumptions and edge cases for_rotate_bbox_to_sync_exported_annotation_with_imported.The logic for determining the best shift is correct and well-organized, but there's no immediate check if
imported_pointshas exactly 4 corners — relying on the caller to handle input validation. Adding a short docstring or inline comment would clarify the requirement that bothimported_pointsandexported_pointsrepresent quadrilaterals and help future maintainers grasp the shift-by-90° increments concept more easily.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/datumaro/plugins/yolo_format/converter.py(2 hunks)src/datumaro/plugins/yolo_format/extractor.py(3 hunks)tests/unit/data_formats/test_yolo_format.py(1 hunks)
🔇 Additional comments (5)
src/datumaro/plugins/yolo_format/extractor.py (2)
48-48: Good move making bbox_annotation_as_polygon accessible.
Importing the newly public function is consistent with the refactor, ensuring reusability across modules.
571-580: Thorough integration of rotation synchronization.
The adjustment of the bounding box attributes post-cv2.minAreaRect() via _rotate_bbox_to_sync_exported_annotation_with_imported is logically coherent, ensuring consistent orientation across imports and exports. The final assignment of rotation attributes appears correct and covers the edge case of near-zero rotation by removing the attribute. Good job.
tests/unit/data_formats/test_yolo_format.py (1)
101-104: Simplified rotation comparison.
Switching from modulo 180 to 360 and removing the extra 90°-handling logic yields a simpler approach that only treats bounding boxes as matching when their rotation differences are negligible (less than 0.01). However, this disallows bounding boxes rotated by 180° or 90° from matching, which might be a breaking change depending on prior requirements. Confirm that this strict rotation check aligns with the new bounding-box sync logic in the extractor.
src/datumaro/plugins/yolo_format/converter.py (2)
49-49: Public function promotes clearer usage.
Renaming _bbox_annotation_as_polygon to bbox_annotation_as_polygon is a good accessibility improvement, making it available for external references. The rotation logic inside is straightforward.
349-349: Good integration of bbox_annotation_as_polygon in oriented boxes converter.
This call consolidates rectangle-to-polygon conversions without code duplication, adhering to DRY principles.
# Conflicts: # src/datumaro/plugins/yolo_format/extractor.py
Co-authored-by: Maxim Zhiltsov <[email protected]>
|
…#8891) <!-- Raise an issue to propose your change (https://github.com/cvat-ai/cvat/issues). It helps to avoid duplication of efforts from multiple independent contributors. Discuss your ideas with maintainers to be sure that changes will be approved and merged. Read the [Contribution guide](https://docs.cvat.ai/docs/contributing/). --> <!-- Provide a general summary of your changes in the Title above --> ### Motivation and context <!-- Why is this change required? What problem does it solve? If it fixes an open issue, please link to the issue here. Describe your changes in detail, add screenshots. --> fixes #8882 depend on cvat-ai/datumaro#72 ### How has this been tested? <!-- Please describe in detail how you tested your changes. Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc. --> ### Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. If an item isn't applicable for some reason, then ~~explicitly strikethrough~~ the whole line. If you don't do that, GitHub will show incorrect progress for the pull request. If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [ ] I submit my changes into the `develop` branch - [ ] I have created a changelog fragment <!-- see top comment in CHANGELOG.md --> - [ ] I have updated the documentation accordingly - [ ] I have added tests to cover my changes - [ ] I have linked related issues (see [GitHub docs]( https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)) - [ ] I have increased versions of npm packages if it is necessary ([cvat-canvas](https://github.com/cvat-ai/cvat/tree/develop/cvat-canvas#versioning), [cvat-core](https://github.com/cvat-ai/cvat/tree/develop/cvat-core#versioning), [cvat-data](https://github.com/cvat-ai/cvat/tree/develop/cvat-data#versioning) and [cvat-ui](https://github.com/cvat-ai/cvat/tree/develop/cvat-ui#versioning)) ### License - [ ] I submit _my code changes_ under the same [MIT License]( https://github.com/cvat-ai/cvat/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Updated `datumaro` dependency to a new commit hash in project requirements files. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
How to test
Checklist
developbranchLicense
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
Refactor
bbox_annotation_as_polygonfunction publicBug Fixes