Add support for loading segmentation datasets in Pascal VOC format #245
Conversation
|
@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 |
…d approx of masks should be in another test suite
|
@SkalskiP , hi, ready for review. |
|
Hi, @kirilllzaitsev 👋🏻! I've done some tests, and I see some problems:
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 |
|
@kirilllzaitsev let me know when you'll be ready ;) |
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. |
|
@SkalskiP , ready |
I think that our current API sucks. Things we can do:
@hardikdava what do you think? |
|
@SkalskiP , I completely agree with you. We can introduce class VisionTask(Enum):
CLASSIFICATION = 0
OBJECT_DETECTION = 1
ORIENTED_BOUNDING_BOX = 2
INSTANCE_SEGMENTATION = 3
KEYPOINTS_DETECTION = 4
POSE_ESITMATION = 5what do you think? |
| ): | ||
| with exception: | ||
| result = object_to_pascal_voc(xyxy=xyxy, name=name, polygon=polygon) | ||
| with open("/tmp/test.xml", "w") as f: |
There was a problem hiding this comment.
@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())There was a problem hiding this comment.
To be honest, I'd drop those four lines.
There was a problem hiding this comment.
An artifact from local tests, apologies
| return polygon_points | ||
|
|
||
|
|
||
| def load_pascal_voc_annotations_v1( |
There was a problem hiding this comment.
Let's drop that logic altogether.
|
@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. |
|
@SkalskiP ready |
|
Looks good to me. Merging! @kirilllzaitsev you plan to work on anything more in this release? |
|
@SkalskiP sure, what else is on the plate? |
|
@kirilllzaitsev, how about splitting |
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.
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