Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for confidence values in object detection labels by updating the in-memory formats, conversion logic, and tests. Key changes include:
- Extending the SingleObjectDetection model to include a confidence field with validation.
- Modifying multiple test suites (unit and integration) to parameterize tests with a "with_confidence" flag.
- Introducing a new CustomObjectDetectionInput format and updating documentation.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/formats/test_pascalvoc.py | Parameterizes test_save with "with_confidence" and updates input generation. |
| tests/unit/formats/test_lightly.py | Adds "with_confidence" parameterization and adjusts expected score values accordingly. |
| tests/unit/formats/test_kitti.py | Adds "with_confidence" support in tests and updates input import. |
| tests/simple_object_detection_label_input.py | Replaces the SimpleObjectDetectionInput class with a get_input() function returning a CustomObjectDetectionInput including confidence. |
| tests/integration/object_detection/test_inverse.py | Applies the "with_confidence" parameterization to inverse conversion tests and adjusts expected output accordingly. |
| src/labelformat/model/object_detection.py | Extends SingleObjectDetection with a confidence field and runtime validation. |
| src/labelformat/formats/lightly.py | Updates label extraction and saving to correctly handle the confidence field. |
| src/labelformat/formats/custom.py | Introduces the CustomObjectDetectionInput and CustomInstanceSegmentationInput classes. |
| docs/formats/object-detection/custom.md | Documents the new custom object detection format and its usage. |
Comments suppressed due to low confidence (2)
src/labelformat/model/object_detection.py:19
- Consider adding unit tests to verify that post_init correctly raises a ValueError when confidence is set to an out-of-range value (i.e. less than 0 or greater than 1).
def __post_init__(self) -> None:
tests/integration/object_detection/test_inverse.py:112
- [nitpick] Document the rationale behind converting a 'None' confidence to 0.0 in the test expectations to clarify the design decision and improve test maintainability.
if obj.confidence is None:
There was a problem hiding this comment.
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/labelformat/formats/custom.py:36
- [nitpick] Consider renaming 'CustomObjectDetectionInput' to 'LabelformatObjectDetectionInput' to align with internal naming conventions as suggested in the PR description.
class CustomObjectDetectionInput(_CustomBaseInput, ObjectDetectionInput):
|
/review |
IgorSusmelj
left a comment
There was a problem hiding this comment.
Only small comments. Otherwise looks good.
| for pil_image, filename in my_dataloader: | ||
| # Create Image object | ||
| current_image = Image( | ||
| id=image_id_counter, |
There was a problem hiding this comment.
Do we need the id here? What is it for?
There was a problem hiding this comment.
It is a required attribute of the labelformat class Image. It is required e.g. when converting to COCO.
If we wanted to change that, we would need to rework many other parts of the code, which belongs to a different PR.
|
|
||
| # Create SingleObjectDetection objects | ||
| objects = [] | ||
| for pred in model_predictions: |
There was a problem hiding this comment.
Maybe list comp here as well?
Smth like
objects = [
SingleObjectDetection(
category=categories[pred['category_id']],
box=BoundingBox.from_format(
box=pred['box'],
format=BoundingBoxFormat(prediction_bbox_format),
),
confidence=pred.get('confidence'),
)
for pred in model_predictions
]
There was a problem hiding this comment.
As this is to be used by the user, I would keep the current for-loop. E.g. it might be that the user needs to do some custom extra operations as part of converting. Doing them as extra lines in a for loop is much easier than adding them to a list comprehension.
There was a problem hiding this comment.
Pull Request Overview
This pull request adds support for confidence values to object detection labels, ensuring they are correctly propagated and validated across various formats and testing scenarios.
- Added a confidence field to SingleObjectDetection with validation.
- Updated tests across multiple formats (PascalVOC, Lightly, KITTI) and integration tests to handle confidence values.
- Updated documentation and the internal LabelformatObjectDetectionInput to include confidence handling.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/model/test_object_detection.py | Added tests for valid and out-of-bound confidence values in object detection. |
| tests/unit/formats/test_pascalvoc.py | Updated PascalVOC tests to handle the with_confidence parameter. |
| tests/unit/formats/test_lightly.py | Modified Lightly tests to assert correct score values based on confidence. |
| tests/unit/formats/test_kitti.py | Introduced the with_confidence parameter for KITTI tests. |
| tests/simple_object_detection_label_input.py | Refactored input creation to return LabelformatObjectDetectionInput with confidence support. |
| tests/integration/object_detection/test_inverse.py | Adapted inverse tests to account for confidence conversion in output. |
| src/labelformat/model/object_detection.py | Added the optional confidence field with proper range checking in post_init. |
| src/labelformat/formats/lightly.py | Updated score extraction and saving logic to incorporate confidence values. |
| src/labelformat/formats/labelformat.py | Created the internal LabelformatObjectDetectionInput format with confidence support. |
| docs/formats/object-detection/labelformat.md | Updated documentation to include details and examples for using confidence values. |
Description
LabelformatObjectDetectionInputfor programmatic creation of that format such that labels are included. Include docs example for creating it in a dataloader for-loop.Open points
Tests
LabelfromatDetectionInputis tested a lot by being the input to the other tests.