DetectionDataset merge fails when class name contains capital letter

Suhas-G commented 4 hours ago

Search before asking

[X] I have searched the Supervision issues and found no similar bug report.

Bug

Hello, thanks for this great library! I'm facing an issue while trying to merge 2 datasets when any of the class names contain a capital letter.

Error:

ValueError: Class Animal not found in target classes. source_classes must be a subset of target_classes.

The issue stems from the merge_class_lists function at https://github.com/roboflow/supervision/blob/37cacec70443a2c28ea6642f6bc54e6c5151c111/supervision/dataset/utils.py#L53 where the class names are converted to lower-case, but build_class_index_mapping keeps the class names as it is. For my use case, I was able to get around by removing the lower-case conversion.

Environment

Supervision 0.24.0
OS: Windows 10
Python 3.10.14

Minimal Reproducible Example

Example: I downloaded 2 roboflow datasets - https://universe.roboflow.com/cvlab-6un5p/cv-lab-kpdek and https://universe.roboflow.com/padidala-indhu-e1dhl/animals-gzsxr and tried to merge them

import supervision as sv

def main():
    ds1 = sv.DetectionDataset.from_coco("data/CV-LAB.v1i.coco/train", "data/CV-LAB.v1i.coco/train/_annotations.coco.json")
    ds2 = sv.DetectionDataset.from_coco("data/Animals.v1i.coco/train", "data/Animals.v1i.coco/train/_annotations.coco.json")

    sv.DetectionDataset.merge([ds1, ds2])

if __name__ == '__main__':
    main()

Additional

No response

Are you willing to submit a PR?

[X] Yes I'd like to help by submitting a PR!

LinasKo commented 4 hours ago

Hi @Suhas-G 👋

We'd like to treat classes with different capitalization as different classes. That is, "Animal" and "animal" are different.

Right now I expect people to rename these manually inside Detections. However, adding a Detections.rename_class(from: str, to: str) and DetectionsDataset.rename_class(from: str, to: str) would be very useful!

Suhas-G commented 4 hours ago

Hi @LinasKo, to treat classes with different capitalization as different classes, isn't it better to not call .lower() in the merge function? That way the merged list will have "Animal" as a different class. Note that in this example, there is no "animal" class but only "Animal" which is converted to lower case in merged list, and so "Animal" is not found while building the index mapping.

LinasKo commented 3 hours ago

Are we calling that? If so, I believe we can remove it.

roboflow / supervision