-
Notifications
You must be signed in to change notification settings - Fork 605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for exports using existing COCO IDs #4530
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant COCODetectionDatasetExporter
participant System
User->>COCODetectionDatasetExporter: Initialize with coco_id parameter
COCODetectionDatasetExporter->>System: Store coco_id and prepare _image_id_map
User->>COCODetectionDatasetExporter: Invoke log_collection with dataset
COCODetectionDatasetExporter->>System: Use coco_id field for image ID mapping
User->>COCODetectionDatasetExporter: Call export_sample for image export
COCODetectionDatasetExporter->>System: Retrieve and map image ID using coco_id
System->>COCODetectionDatasetExporter: Return mapped image ID
COCODetectionDatasetExporter->>User: Complete image export with updated image ID
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range and nitpick comments (6)
fiftyone/utils/coco.py (6)
Line range hint
824-827
: Optimize conditional assignment using a ternary operator.The assignment of
file_name
based onself.abs_paths
can be simplified using a ternary operator for better readability.- if self.abs_paths: - file_name = out_image_path - else: - file_name = uuid + file_name = out_image_path if self.abs_paths else uuidTools
Ruff
833-834: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
836-836: No explicit
stacklevel
keyword argument found (B028)
Line range hint
970-970
: Remove unnecessary inheritance fromobject
.In Python 3, all classes implicitly inherit from
object
, so it's redundant to specify it.- class COCOObject(object): + class COCOObject:
Line range hint
1224-1227
: Simplify attribute assignment with a ternary operator.The assignment of
attributes
can be simplified using a ternary operator to enhance code readability.- if extra_attrs: - attributes = {f: d.get(f, None) for f in extra_attrs} - else: - attributes = {} + attributes = {f: d.get(f, None) for f in extra_attrs} if extra_attrs else {}
Line range hint
1319-1319
: Use format specifiers instead of percent format.The error message uses percent formatting. It's recommended to use Python's
.format()
or f-strings for better readability and performance.- "Unsupported label type '%s'. Supported types are %s" - % (type(label), self.label_cls) + f"Unsupported label type '{type(label)}'. Supported types are {self.label_cls}"
Line range hint
1332-1335
: Simplify conditional assignment using a ternary operator.The assignment of
_id
can be simplified using a ternary operator for better readability.- if id_attr is not None: - _id = label.get_attribute_value(id_attr, None) - else: - _id = None + _id = label.get_attribute_value(id_attr, None) if id_attr is not None else None
Line range hint
1367-1370
: Use a ternary operator to simplify label assignment.The assignment of
label
based onclasses_map
can be optimized using a ternary operator.- if classes_map: - label = classes_map[self.category_id] - else: - label = str(self.category_id) + label = classes_map[self.category_id] if classes_map else str(self.category_id)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- fiftyone/utils/coco.py (6 hunks)
Additional context used
Ruff
fiftyone/utils/coco.py
245-246: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
605-605: Ambiguous variable name:
l
(E741)
824-827: Use ternary operator
file_name = out_image_path if self.abs_paths else uuid
instead ofif
-else
-block (SIM108)Replace
if
-else
-block withfile_name = out_image_path if self.abs_paths else uuid
833-834: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
836-836: No explicit
stacklevel
keyword argument found (B028)
868-869: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
881-882: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
884-884: No explicit
stacklevel
keyword argument found (B028)
939-939: Ambiguous variable name:
l
(E741)
970-970: Class
COCOObject
inherits fromobject
(UP004)Remove
object
inheritance
1224-1227: Use ternary operator
attributes = {f: d.get(f, None) for f in extra_attrs} if extra_attrs else {}
instead ofif
-else
-block (SIM108)Replace
if
-else
-block withattributes = {f: d.get(f, None) for f in extra_attrs} if extra_attrs else {}
1319-1319: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1332-1335: Use ternary operator
_id = label.get_attribute_value(id_attr, None) if id_attr is not None else None
instead ofif
-else
-block (SIM108)Replace
if
-else
-block with_id = label.get_attribute_value(id_attr, None) if id_attr is not None else None
1367-1370: Use ternary operator
label = classes_map[self.category_id] if classes_map else str(self.category_id)
instead ofif
-else
-block (SIM108)Replace
if
-else
-block withlabel = classes_map[self.category_id] if classes_map else str(self.category_id)
1562-1563: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1568-1569: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1812-1815: Use ternary operator
label_types = [label_types] if etau.is_str(label_types) else list(label_types)
instead ofif
-else
-block (SIM108)Replace
if
-else
-block withlabel_types = [label_types] if etau.is_str(label_types) else list(label_types)
1817-1817: Ambiguous variable name:
l
(E741)
1821-1822: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1827-1828: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1838-1839: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1842-1842: Avoid equality comparisons to
True
; useif include_license:
for truth checks (E712)Replace with
include_license
1921-1922: Use a single
with
statement with multiple contexts instead of nestedwith
statements (SIM117)
1944-1944: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1978-1979: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
2005-2005: Unnecessary open mode parameters (UP015)
Remove open mode parameters
2006-2006: Ambiguous variable name:
l
(E741)
2010-2010: Unnecessary open mode parameters (UP015)
Remove open mode parameters
2059-2062: Use ternary operator
classes = {classes} if etau.is_str(classes) else set(classes)
instead ofif
-else
-block (SIM108)Replace
if
-else
-block withclasses = {classes} if etau.is_str(classes) else set(classes)
2149-2149: Avoid equality comparisons to
True
; useif extra_attrs:
for truth checks (E712)Replace with
extra_attrs
2152-2152: Avoid equality comparisons to
False
; useif not extra_attrs:
for false checks (E712)Replace with
not extra_attrs
2274-2274: Do not use bare
except
(E722)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🙇
Adds an optional
coco_id
parameter that can be passed whenexport()
ing in COCO format that preserves existing COCO image IDs that are stored on a field of your dataset.Note that, as the example below shows, we already support storing COCO IDs upon import by passing the
include_id=True
parameter.Summary by CodeRabbit