Add support for loading segmentation datasets in Pascal VOC format

[kirilllzaitsev]

Description

Add possibility to load PASCAL VOC segmentation masks in addition to object detection (related issue).
Changes were made to core.DetectionDataset.from_pascal_voc and format.pascal_voc.load_pascal_voc_annotations.

Type of change

Please delete options that are not relevant.

• [+] New feature (non-breaking change which adds functionality)

How has this change been tested, please provide a testcase or example of how you tested the change?

Via new tests.

Any specific deployment considerations

For example, documentation changes, usability, usage/costs, secrets, etc.

Docs

• Docs updated? What were the changes:

[kirilllzaitsev]

@SkalskiP , new implementation is not compatible with v1 of load_pascal_voc_annotations, but matches the loading standard of YOLO and COCO. Shall we temporarily leave the v1 there and issue a warning, or change the implementation to be backward compatible? Is there a general rule that we follow for such cases?

[SkalskiP]

new implementation is not compatible with v1 of load_pascal_voc_annotations, but matches the loading standard of YOLO and COCO. Shall we temporarily leave the v1 there and issue a warning, or change the implementation to be backward compatible? Is there a general rule that we follow for such cases?

@kirilllzaitsev don't worry about breaking old load_pascal_voc_annotations. It is a function that is not publicly exposed.

[kirilllzaitsev]

@SkalskiP , hi, ready for review.

[SkalskiP]

Hi, @kirilllzaitsev 👋🏻! I've done some tests, and I see some problems:

• from_pascal_voc loads mask data even if force_masks=False.
• mask.shape have incorrect order of channels. It is (N, W, H). It should be (N, H, W).
• mask.dtype is incorrect. It is uint8. It should be bool.

Here is a Google Colab you can use to verify my findings: https://colab.research.google.com/drive/18rDUnAwPxhMt9YVbnFof5MS6__59NL11?usp=sharing

I'm not sure yet what to do with force_masks. I see there is inconsistency across different format loaders.

[SkalskiP]

@kirilllzaitsev let me know when you'll be ready ;)

[kirilllzaitsev]

Hi, @kirilllzaitsev 👋🏻! I've done some tests, and I see some problems:

• from_pascal_voc loads mask data even if force_masks=False.
• mask.shape have incorrect order of channels. It is (N, W, H). It should be (N, H, W).
• mask.dtype is incorrect. It is uint8. It should be bool.

Here is a Google Colab you can use to verify my findings: https://colab.research.google.com/drive/18rDUnAwPxhMt9YVbnFof5MS6__59NL11?usp=sharing

I'm not sure yet what to do with force_masks. I see there is inconsistency across different format loaders.

To me, setting the force_masks flag translates into 'masks are required, and if there are no masks - raise an error'. This is what the YOLO loader does, contrary to the COCO that uses force_masks as an indicator of whether to use masks or not. The latter seems confusing to me.

[kirilllzaitsev]

@SkalskiP , ready

[SkalskiP]

To me, setting the force_masks flag translates into 'masks are required, and if there are no masks - raise an error'. This is what the YOLO loader does, contrary to the COCO that uses force_masks as an indicator of whether to use masks or not. The latter seems confusing to me.

I think that our current API sucks. force_masks is completely non-intuitive. And of course, this is my fault. :) I just think about how to fix it.

Things we can do:

• Keep force_masks but make this behavior consistent for all formats.
• Have separate methods for detection and segmentation. For example from_coco and from_coco_cegemtnation.

@hardikdava what do you think?

[hardikdava]

@SkalskiP , I completely agree with you. We can introduce sv.VisionTask as argument to sv.DetectionDataset for scalable and specifying types which can be standardize.

class VisionTask(Enum):
	CLASSIFICATION = 0
	OBJECT_DETECTION = 1
	ORIENTED_BOUNDING_BOX = 2
	INSTANCE_SEGMENTATION = 3
	KEYPOINTS_DETECTION = 4
	POSE_ESITMATION = 5

what do you think?

[SkalskiP]

@kirilllzaitsev, what are we testing here? 👇🏻

with open("/tmp/test.xml", "w") as f:
    f.write(ET.tostring(result).decode())
with open("/tmp/exptest.xml", "w") as f:
    f.write(ET.tostring(expected_result).decode())

[SkalskiP]

To be honest, I'd drop those four lines.

[kirilllzaitsev]

An artifact from local tests, apologies

[SkalskiP]

Let's drop that logic altogether.

[SkalskiP]

@kirilllzaitsev I left a few more final comments. But we are definitely on the right path.

Also, please makes sure to run 👇🏻 before the final commit.

isort --profile black supervision/
black supervision

[kirilllzaitsev]

@SkalskiP ready

[SkalskiP]

Looks good to me. Merging! @kirilllzaitsev you plan to work on anything more in this release?

[kirilllzaitsev]

@SkalskiP sure, what else is on the plate?

[SkalskiP]

@kirilllzaitsev, how about splitting DetectionDataset into DetectionDataset and SegmentationDataset POC? #244