Repository navigation
Conversation
Stacked on the data args PR. Adds: - COCO and YOLO KeypointDetectionDatasetArgs with list_image_info - KeypointDetectionDataset, __getitem__ still unimplemented - open_yolo_keypoint_detection_label, with file:line error messages - KeypointDetectionDatasetItem and the two NDArray aliases Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83307226ed
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| vis = int(values[offset + 2]) | ||
| if vis not in keypoint_helpers.VISIBILITIES: |
There was a problem hiding this comment.
For a malformed label containing a fractional visibility such as 1.9, this conversion truncates it to 1, so the subsequent membership check silently accepts it as occluded instead of rejecting the invalid annotation. Compare the parsed float against its integer conversion before accepting it while still allowing representations such as 2.000000; the COCO parser's analogous conversion should be handled consistently.
Useful? React with 👍 / 👎.
8330722 to
29a0139
Compare
Reject fractional visibility flags instead of truncating them: 1.9 no longer passes as occluded, while 2.000000 still parses. Shared by the COCO and YOLO parsers. Move the private helpers to the bottom of the module and document the convention in CONTRIBUTING.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
/review |
Replace the VISIBILITY_* constants and the VISIBILITIES tuple with a Visibility IntEnum. Values stay ints, so they still serialize into the mmap columns and compare against the ints read back from it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gabrielfruet
left a comment
gabrielfruet
left a comment
There was a problem hiding this comment.
I'm only concerned about the COCO, but the rest looks good
| return len(next(iter(distinct))) | ||
|
|
||
|
|
||
| def bbox_from_keypoints( |
There was a problem hiding this comment.
Nit: np array may be faster here, since keypoints_xy can be represented as a regular tensor, non-ragged. But we don't need to care too much about this now.
|
|
||
| image_dir = file_helpers.resolve_coco_images_dir(self.labels, self.data_dir) | ||
|
|
||
| for image in labels_dict["images"]: |
There was a problem hiding this comment.
Shouldn't this logic go into file helpers just like YOLO?
What has changed and why?
Adds COCO and YOLO keypoint dataset args and label readers on top of #977.
How has it been tested?
uv run --frozen pytest tests/_data/test_keypoint_detection_dataset.py tests/_data/test_keypoint_helpers.pyDid you update CHANGELOG.md?
Did you update the documentation?