Add support for annotating frame views #4477
Conversation
WalkthroughThe changes in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AnnotateFunction
participant FinalizeIDMap
participant LoadAnnotations
participant MergeScalarsAndLabels
User->>AnnotateFunction: Call annotate()
AnnotateFunction->>FinalizeIDMap: Call _finalize_id_map()
AnnotateFunction->>LoadAnnotations: Call load_annotations()
LoadAnnotations->>MergeScalarsAndLabels: Call _merge_scalars() and _merge_labels()
AnnotateFunction->>User: Return results
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (9)
fiftyone/utils/annotations.py (9)
Line range hint
60-60: Replace percent format with format specifiers for better readability and performance.- raise ValueError("The '%s' backend does not expose an API" % backend) + raise ValueError(f"The '{backend}' backend does not expose an API")
Line range hint
573-573: Replace equality comparison toTruewith a direct truth check.- if F("filled") == True: - return samples.filter_labels(label_field, F("filled"), only_matches=False) + if F("filled"): + return samples.filter_labels(label_field, F("filled"), only_matches=False)
Line range hint
578-578: Replace equality comparison toFalsewith a direct falsity check.- if F("filled") == False: - return samples.filter_labels(label_field, not F("filled"), only_matches=False) + if not F("filled"): + return samples.filter_labels(label_field, not F("filled"), only_matches=False)
Line range hint
822-825: Use a ternary operator for a more concise and readable assignment.- if etau.is_str(classes): - classes = [classes] - else: - classes = list(classes) + classes = [classes] if etau.is_str(classes) else list(classes)
Line range hint
878-878: Replace equality comparison toTruewith a direct truth check.- if attributes == True: - attributes = _get_label_attributes(samples, backend, label_field, label_type, classes=classes) + if attributes: + attributes = _get_label_attributes(samples, backend, label_field, label_type, classes=classes)
Line range hint
1045-1045: Simplify the method call by removing the unnecessary default parameter.- expected_type = _RETURN_TYPES_MAP.get(label_type, None) + expected_type = _RETURN_TYPES_MAP.get(label_type)
Line range hint
1362-1365: Use a ternary operator for a more concise and readable assignment.- if is_frame_field: - images = sample.frames.values() - else: - images = [sample] + images = sample.frames.values() if is_frame_field else [sample]Tools
Ruff
1357-1360: Use ternary operator
sample_annos = anno_dict if is_frames_view else anno_dict.get(sample.id, None)instead ofif-else-block (SIM108)Replace
if-else-block withsample_annos = anno_dict if is_frames_view else anno_dict.get(sample.id, None)
Line range hint
1603-1606: Use a ternary operator for a more concise and readable assignment.- if is_list: - labels = image_label[list_field] - else: - labels = [image_label] + labels = image_label[list_field] if is_list else [image_label]
Line range hint
2337-2337: Remove unnecessary inheritance fromobjectin Python 3.- class AnnotationAPI(object): + class AnnotationAPI:
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- fiftyone/utils/annotations.py (26 hunks)
Additional context used
Ruff
fiftyone/utils/annotations.py
60-60: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
206-207: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
220-221: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
277-278: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
289-289: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
429-436: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
446-449: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
453-456: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
573-573: Avoid equality comparisons to
True; useif F("filled"):for truth checks (E712)Replace with
F("filled")
578-578: Avoid equality comparisons to
False; useif not F("filled"):for false checks (E712)Replace with
not F("filled")
582-583: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
636-637: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
648-650: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
669-670: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
722-729: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
764-764: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
802-803: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
817-817: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
822-825: Use ternary operator
classes = [classes] if etau.is_str(classes) else list(classes)instead ofif-else-block (SIM108)Replace
if-else-block withclasses = [classes] if etau.is_str(classes) else list(classes)
878-878: Avoid equality comparisons to
True; useif attributes:for truth checks (E712)Replace with
attributes
938-938: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
943-950: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
960-961: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
968-969: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1045-1045: Use
_RETURN_TYPES_MAP.get(label_type)instead of_RETURN_TYPES_MAP.get(label_type, None)(SIM910)Replace
_RETURN_TYPES_MAP.get(label_type, None)with_RETURN_TYPES_MAP.get(label_type)
1223-1226: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1230-1232: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1273-1273: Do not use bare
except(E722)
1292-1294: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1298-1300: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1357-1360: Use ternary operator
sample_annos = anno_dict if is_frames_view else anno_dict.get(sample.id, None)instead ofif-else-block (SIM108)Replace
if-else-block withsample_annos = anno_dict if is_frames_view else anno_dict.get(sample.id, None)
1362-1365: Use ternary operator
images = sample.frames.values() if is_frame_field else [sample]instead ofif-else-block (SIM108)Replace
if-else-block withimages = sample.frames.values() if is_frame_field else [sample]
1378-1378: Do not use bare
except(E722)
1494-1494: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
1498-1498: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
1557-1560: Use ternary operator
sample_annos = anno_dict if is_frames_view else anno_dict[sample_id]instead ofif-else-block (SIM108)Replace
if-else-block withsample_annos = anno_dict if is_frames_view else anno_dict[sample_id]
1562-1565: Use ternary operator
images = sample.frames.values() if is_frame_field else [sample]instead ofif-else-block (SIM108)Replace
if-else-block withimages = sample.frames.values() if is_frame_field else [sample]
1603-1606: Use ternary operator
labels = image_label[list_field] if is_list else [image_label]instead ofif-else-block (SIM108)Replace
if-else-block withlabels = image_label[list_field] if is_list else [image_label]
2337-2337: Class
AnnotationAPIinherits fromobject(UP004)Remove
objectinheritance
2352-2352: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
2371-2371: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
There was a problem hiding this comment.
Actionable comments posted: 7
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- fiftyone/utils/annotations.py (25 hunks)
Additional comments not posted (2)
fiftyone/utils/annotations.py (2)
1346-1349: Utilize existing comment to improve code readability.The existing comment from a previous review already suggests using a ternary operator here for more concise code. Consider applying this change to improve readability.
1037-1037: Check the initialization ofis_frames_view.#!/bin/bash # Description: Verify if `is_frames_view` is properly initialized before use. # Test: Search for the initialization of `is_frames_view`. Expect: Initialization before use. rg --type python --multiline $'is_frames_view =.*\n.*results._is_frames'
brimoor
force-pushed
the
annotate-frame-views
branch
from
b77da70
to
b0ba35c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- fiftyone/utils/annotations.py (25 hunks)
Additional context used
Ruff
fiftyone/utils/annotations.py
60-60: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
206-207: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
220-221: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
277-278: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
289-289: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
429-436: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
446-449: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
453-456: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
573-573: Avoid equality comparisons to
True; useif F("filled"):for truth checks (E712)Replace with
F("filled")
578-578: Avoid equality comparisons to
False; useif not F("filled"):for false checks (E712)Replace with
not F("filled")
582-583: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
636-637: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
648-650: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
669-670: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
722-729: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
764-764: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
802-803: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
817-817: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
822-825: Use ternary operator
classes = [classes] if etau.is_str(classes) else list(classes)instead ofif-else-block (SIM108)Replace
if-else-block withclasses = [classes] if etau.is_str(classes) else list(classes)
878-878: Avoid equality comparisons to
True; useif attributes:for truth checks (E712)Replace with
attributes
938-938: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
943-950: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
960-961: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
968-969: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1045-1045: Use
_RETURN_TYPES_MAP.get(label_type)instead of_RETURN_TYPES_MAP.get(label_type, None)(SIM910)Replace
_RETURN_TYPES_MAP.get(label_type, None)with_RETURN_TYPES_MAP.get(label_type)
1212-1215: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1219-1221: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1262-1262: Do not use bare
except(E722)
1281-1283: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1287-1289: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
1346-1349: Use ternary operator
sample_annos = anno_dict if is_frames_view else anno_dict.get(sample.id, None)instead ofif-else-block (SIM108)Replace
if-else-block withsample_annos = anno_dict if is_frames_view else anno_dict.get(sample.id, None)
1351-1354: Use ternary operator
images = sample.frames.values() if is_frame_field else [sample]instead ofif-else-block (SIM108)Replace
if-else-block withimages = sample.frames.values() if is_frame_field else [sample]
1367-1367: Do not use bare
except(E722)
1483-1483: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
1487-1487: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()
1546-1549: Use ternary operator
sample_annos = anno_dict if is_frames_view else anno_dict[sample_id]instead ofif-else-block (SIM108)Replace
if-else-block withsample_annos = anno_dict if is_frames_view else anno_dict[sample_id]
1551-1554: Use ternary operator
images = sample.frames.values() if is_frame_field else [sample]instead ofif-else-block (SIM108)Replace
if-else-block withimages = sample.frames.values() if is_frame_field else [sample]
1592-1595: Use ternary operator
labels = image_label[list_field] if is_list else [image_label]instead ofif-else-block (SIM108)Replace
if-else-block withlabels = image_label[list_field] if is_list else [image_label]
2338-2338: Class
AnnotationAPIinherits fromobject(UP004)Remove
objectinheritance
2353-2353: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
2372-2372: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
Additional comments not posted (8)
fiftyone/utils/annotations.py (8)
250-251: Ensure proper documentation for_finalize_id_mapmethod.
1062-1064: Refactor to handle frame fields more efficiently.
1098-1102: Simplify the handling of unexpected labels.
1151-1156: Optimize the handling of unexpected annotations for frame views.
1204-1209: Ensure input validation for user-provided field names.
1314-1320: Clarify the mapping of frame IDs to sample IDs.
1346-1349: Use a ternary operator for a more concise and readable assignment.Tools
Ruff
1346-1349: Use ternary operator
sample_annos = anno_dict if is_frames_view else anno_dict.get(sample.id, None)instead ofif-else-block (SIM108)Replace
if-else-block withsample_annos = anno_dict if is_frames_view else anno_dict.get(sample.id, None)
2236-2248: Document the_finalize_id_mapmethod.
Adds support for passing frame views to
annotate().No changes are necessary in the annotation backends themselves to support frame views. From the backend's standpoint it's just like any other image collection is being annotated; the extra bookkeeping is handled automatically via the core SDK 🎉
Summary by CodeRabbit
These changes enhance the annotation functionality, especially for frames views, providing a more streamlined and efficient user experience.