feat: Merge Multiple Polygons in a COCO Dataset Annotation

[DancinParrot]

Description

This PR attempts to address issue #1209. Within an appropriately structured COCO dataset, an annotation may contain a segmentation list with multiple nested lists of vertices. This results in supervision terminating prematurely and raising a ValueError.

The solution as discussed in the issue includes the creation of binary masks for all polygons within the segmentation list and the usage of numpy's logical_or() function to merge all masks which is used in the coco_annotations_to_masks function.

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)
• This change requires a documentation update

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

The change has been tested using a custom COCO dataset which produces the ValueError mentioned in the issue linked above. You may test out the changes by following the guide on the provided Jupyter Notebook using the following resources:

• Google Colab Jupyter Notebook: https://colab.research.google.com/drive/1P57MlFAypF2pvrwIxPQl2hIU8mxoFe7T?usp=sharing
• Custom COCO dataset: https://drive.google.com/file/d/1GHA4z8QK07efOu9Is3v__Dh4V0D483sB/view?usp=sharing

Any specific deployment considerations

Documentation may require an update.

Docs

• Docs updated? What were the changes: Created an entry under docs/datasets/utils.md with description and types of the function and its parameters. An example and type have also been added for its output.

[CLAassistant]

All committers have signed the CLA.

[SkalskiP]

I think the merge_polygons name is a bit misleading and not conststant with outhe util names in supervision. I think we should call it polygons_to_mask.

And of course we already have a function called polygon_to_mask that does the same but only take one mask.

It seems a bit silly that we would have 2 functions doeng almost the same. I thin kwe should:

• rename merge_polygons to polygons_to_mask.
• make sure that polygons_to_mask is not using polygon_to_mask internally.
• modify places where polygon_to_mask was used and use polygons_to_mask instead.
• mask polygon_to_mask as deprecated and recommend using polygons_to_mask instead.

[SkalskiP]

@DancinParrot, here is an example of how we mark functions as deprecated. We use deprecated decorator to do it.

@deprecated(
    "`Detections.from_roboflow` is deprecated and will be removed in "
    "`supervision-0.22.0`. Use `Detections.from_inference` instead."
)

[SkalskiP]

Please use supervision-0.27.0 as a deprecation version.

[SkalskiP]

Hi @DancinParrot 👋🏻 Thanks a lot for your contribution 🙏🏻 This is a valid change. As I try to keep the code clean, I left a comment proposing a rename to polygons_to_mask and deprecation of the polygon_to_mask function. What do you think?

[DancinParrot]

Hi @SkalskiP ! Thanks for reviewing and providing feedback on my PR!

As I try to keep the code clean, I left a comment proposing a rename to polygons_to_mask and deprecation of the polygon_to_mask function. What do you think?

Yup, I think that's a great idea. It'd certainly be a more accurate representation of the function's intended purpose. Since multiple lists of vertices within the "segmentation" field (which in itself is a list) of a COCO dataset annotation is perfectly acceptable, the function should be capable of handling both one and multiple lists without relying on yet another function that introduces another unnecessary layer of abstraction.

I'll proceed with implementing the suggested changes and deprecating the current polygon_to_mask() function in favour of polygons_to_mask().

Thanks again for your feedback, appreciate it!

[DancinParrot]

Hi @SkalskiP ! During refactoring, I noticed a very similar function _polygons_to_mask() (see here). The function follows a similar methadology in calling polygon_to_mask() for each polygon in the input list, though it does not require the merging of masks into one. In this case, given the similarity of the names with a slight difference in terms of functionality, would the introduction of polygons_to_mask() cause confusion?

I suppose we could refactor it in such a way but it does seem to be a bit odd:

def _polygons_to_masks(
    polygons: List[np.ndarray], resolution_wh: Tuple[int, int]
) -> np.ndarray:
    return np.array(
        [
            polygons_to_mask(polygon=[polygon], resolution_wh=resolution_wh)
            for polygon in polygons
        ],
        dtype=bool,
    )

[DancinParrot]

Hi @SkalskiP ! Any updates on the above?