Skip to content
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

🛠️ fix incorrect COCO class id loading #176

Merged
merged 2 commits into from
Jul 5, 2023

Conversation

SkalskiP
Copy link
Collaborator

@SkalskiP SkalskiP commented Jul 4, 2023

Description

Draft of PR fixing issues mentioned #150. It is an alternative to the #169 solution.

@SkalskiP
Copy link
Collaborator Author

SkalskiP commented Jul 4, 2023

Hi, @hardikdava 👋🏻 did you have a chance to test this?

@hardikdava
Copy link
Collaborator

@SkalskiP Let me test and confirm your fix.

@hardikdava
Copy link
Collaborator

hardikdava commented Jul 4, 2023

@SkalskiP Test summary:

Case-1: from_coco() -> to_coco()
Works fine if image_id and labels_id starts with 0

Case-2: from_yolo() -> to_coco()
image_id and labels_id starts with 0 after saving as coco format. But no mixed up in categories.

@SkalskiP SkalskiP added the version: 0.12.0 Feature to be added in `0.12.0` release label Jul 4, 2023
@SkalskiP
Copy link
Collaborator Author

SkalskiP commented Jul 4, 2023

@hardikdava, as for case 2, do you think we would like image_id and labels_id to start from 1?

@hardikdava
Copy link
Collaborator

@SkalskiP Yes, based on my research and description from coco, opencv, etc it is their standard format to start image-id and label-id starting from 1. Once this is fixed then we are good to go.

Copy link
Collaborator

@hardikdava hardikdava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this small changes and we can close this.

supervision/dataset/formats/coco.py Show resolved Hide resolved
@SkalskiP
Copy link
Collaborator Author

SkalskiP commented Jul 5, 2023

@hardikdava, that should be easy. I'll update .to_coco logic before merging.

@SkalskiP
Copy link
Collaborator Author

SkalskiP commented Jul 5, 2023

@hardikdava given the fact that @yclicc tested the code and it works as expected. (read more here #150) I'll probably close #169. Is it okey with you?

@hardikdava
Copy link
Collaborator

@SkalskiP Yes, please mark it as completed.

@SkalskiP SkalskiP marked this pull request as ready for review July 5, 2023 09:38
@SkalskiP SkalskiP merged commit abbde88 into main Jul 5, 2023
@SkalskiP SkalskiP added this to the version: 0.12.0 milestone Jul 21, 2023
@SkalskiP SkalskiP self-assigned this Jul 21, 2023
@SkalskiP SkalskiP deleted the fix/coco_incorrect_class_id_processing branch August 8, 2023 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version: 0.12.0 Feature to be added in `0.12.0` release
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants