-
Notifications
You must be signed in to change notification settings - Fork 11
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
COCO format support #102
base: main
Are you sure you want to change the base?
COCO format support #102
Conversation
WalkthroughAn update to the SLEAP IO module enhanced functionality for handling COCO datasets, including reading, writing, and generating labels. Additionally, new methods for video handling and metadata integration were introduced, with improvements made to frame reading logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SLEAP as SLEAP IO
participant COCOParser as COCO Parser
participant Video
User->>SLEAP: Request to load COCO dataset
SLEAP->>COCOParser: Call load_coco with filename and paths
COCOParser->>Video: Generate video instances
COCOParser->>User: Return parsed Labels
Poem
Tip AI model upgrade
|
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: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- sleap_io/io/coco.py (1 hunks)
- sleap_io/io/main.py (5 hunks)
- sleap_io/io/video.py (1 hunks)
- sleap_io/model/video.py (3 hunks)
Additional context used
Ruff
sleap_io/io/coco.py
20-23: Use
contextlib.suppress(ImportError)
instead oftry
-except
-pass
Replace with
contextlib.suppress(ImportError)
(SIM105)
35-35: Unnecessary open mode parameters
Remove open mode parameters
(UP015)
82-82: Use
is
andis not
for type comparisons, orisinstance()
for isinstance checks(E721)
152-152: Use
ann.get("info")
instead ofann.get("info", None)
Replace
ann.get("info", None)
withann.get("info")
(SIM910)
223-223: Undefined name
video
(F821)
sleap_io/model/video.py
63-63: Use
X | Y
for type annotationsConvert to
X | Y
(UP007)
65-65: Use
X | Y
for type annotationsConvert to
X | Y
(UP007)
66-66: Use
X | Y
for type annotationsConvert to
X | Y
(UP007)
Additional comments not posted (8)
sleap_io/io/main.py (2)
134-168
: LGTM!The code changes for loading COCO datasets are approved.
228-229
: LGTM!The code changes for handling COCO files in
load_file
are approved.sleap_io/io/coco.py (4)
40-58
: LGTM!The code changes for parsing skeleton metadata are approved.
157-178
: LGTM!The code changes for reading and parsing COCO annotations are approved.
129-129
: Useisinstance
for type comparison.Use
isinstance
instead oftype
for type comparisons.- pts[pts[:, 3] != 2] = np.nan + pts[pts[:, 2] != 2] = np.nanLikely invalid or redundant comment.
92-92
: Verify the usage ofann["id"]
.Ensure that
ann["id"]
is correctly defined and used.Verification successful
ann["id"]
is correctly defined and used.The key
ann["id"]
is assigned a value frominst_id
and is used appropriately within the codebase.
sleap_io/io/coco.py
:ann["id"] = inst_id
sleap_io/io/coco.py
:video_id_map[ann["id"]] = (
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `ann["id"]`. # Test: Search for the definition and usage of `ann["id"]`. Expect: Proper definition and usage. rg --type python $'ann["id"]'Length of output: 61
Script:
#!/bin/bash # Description: Verify the usage of `ann["id"]`. # Test: Search for the definition and usage of `ann["id"]` in Python files. rg --type py $'ann["id"]'Length of output: 26
Script:
#!/bin/bash # Description: Verify the usage of `ann`. # Test: Search for the definition and usage of `ann` in Python files. rg --type py '\bann\b'Length of output: 2263
sleap_io/model/video.py (1)
Line range hint
66-88
: LGTM!The code changes for adding
backend_metadata
tofrom_filename
are approved.sleap_io/io/video.py (1)
740-743
: LGTM!The code changes for conditionally using
cv2.imread
in_read_frame
are approved.
@@ -131,6 +131,43 @@ def save_jabs(labels: Labels, pose_version: int, root_folder: Optional[str] = No | |||
jabs.write_labels(labels, pose_version, root_folder) | |||
|
|||
|
|||
def load_coco( |
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.
Reminder: Address the TODO.
The TODO comment indicates that handling multiple annotations is not yet implemented.
Do you want me to implement the recursive call for handling multiple annotations or open a GitHub issue to track this task?
This function will group images that have the same shape together into a single | ||
logical video. | ||
""" | ||
if type(imgs_prefix) == str: |
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.
Use isinstance
for type comparison.
Use isinstance
instead of type
for type comparisons.
- if type(imgs_prefix) == str:
+ if isinstance(imgs_prefix, str):
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if type(imgs_prefix) == str: | |
if isinstance(imgs_prefix, str): |
Tools
Ruff
82-82: Use
is
andis not
for type comparisons, orisinstance()
for isinstance checks(E721)
Use isinstance
for type comparison.
Use isinstance
instead of type
for type comparisons.
- if type(imgs_prefix) == str:
+ if isinstance(imgs_prefix, str):
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if type(imgs_prefix) == str: | |
if isinstance(imgs_prefix, str): |
Tools
Ruff
82-82: Use
is
andis not
for type comparisons, orisinstance()
for isinstance checks(E721)
img_filename_map = {} | ||
for img_id, lf in enumerate(lfs): | ||
img_filename = f"{img_id}.{img_format}" | ||
img_shape = video.shape[[1, 2]] |
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.
Fix the undefined variable video
.
The variable video
is undefined.
- img_shape = video.shape[[1, 2]]
+ img_shape = lf.video.shape[[1, 2]]
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
img_shape = video.shape[[1, 2]] | |
img_shape = lf.video.shape[[1, 2]] |
Tools
Ruff
223-223: Undefined name
video
(F821)
Returns: | ||
A dictionary with the parsed data. | ||
""" | ||
with open(ann_json_path, "r") as f: |
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.
Remove unnecessary open mode parameter.
The open mode parameter "r" is unnecessary.
- with open(ann_json_path, "r") as f:
+ with open(ann_json_path) as f:
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
with open(ann_json_path, "r") as f: | |
with open(ann_json_path) as f: |
Tools
Ruff
35-35: Unnecessary open mode parameters
Remove open mode parameters
(UP015)
Summary by CodeRabbit
New Features
load_coco
function.from_filename
method to include backend-specific metadata.Improvements
cv2.imread
oriio.imread
based on module availability.Bug Fixes
load_file
function to handle COCO file format appropriately.