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

[Dataset manager] Fix import for MOTS format #3612

Merged
merged 10 commits into from
Sep 1, 2021
Merged

Conversation

sizov-kirill
Copy link
Contributor

@sizov-kirill sizov-kirill commented Aug 27, 2021

Motivation and context

Fixed #3360.
Some masks in the MOTS PNG format may contain several polygons with the same track_id, for such annotations to be loaded into CVAT, they must be grouped during export. This is what I did in this PR.

How has this been tested?

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

Copy link
Contributor

@zhiltsov-max zhiltsov-max left a comment

Choose a reason for hiding this comment

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

Is it the only format affected?

@sizov-kirill
Copy link
Contributor Author

Is it the only format affected?

I think yes, because this is the only format for which we use transform('masks_to_polygons') and which supports track_id.

@sizov-kirill sizov-kirill changed the title [Dataset manager] Fix export for MOTS format WIP [Dataset manager] Fix export for MOTS format Aug 27, 2021
@sizov-kirill sizov-kirill changed the title WIP [Dataset manager] Fix export for MOTS format WIP [Dataset manager] Fix import for MOTS format Aug 27, 2021
zhiltsov-max
zhiltsov-max previously approved these changes Aug 30, 2021
Copy link
Contributor

@zhiltsov-max zhiltsov-max left a comment

Choose a reason for hiding this comment

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

Looks working, but I think, it deserves a test.

@sizov-kirill
Copy link
Contributor Author

Looks working, but I think, it deserves a test.

Yes is it, but current CVAT tests check if CVAT can upload annotations dumped by CVAT.
Should we run tests for uploading different various of external datasets in supported formats?

Comment on lines 925 to 927
from datumaro.components.dataset import Dataset, DatasetItem
from datumaro.components.extractor import Mask
from numpy import array
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the point of such import?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These classes are only needed in this test, so I decided to import them inside the function.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not a significant reason for having them here.

Also, numpy is typically imported as import numpy as np

@sizov-kirill sizov-kirill changed the title WIP [Dataset manager] Fix import for MOTS format [Dataset manager] Fix import for MOTS format Sep 1, 2021
@@ -908,7 +908,7 @@ def _make_image(i, **kwargs):
attributes["labels"].append({"label_id": idx, "name": label["name"], "color": label["color"]})
attributes["track_id"] = -1

dm_item = datumaro.DatasetItem(id=osp.split(frame_data.name)[-1].split('.')[0],
dm_item = datumaro.DatasetItem(id=osp.splitext(osp.split(frame_data.name)[-1])[0],
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably, this should not be in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll made these changes in new PR

@bsekachev bsekachev deleted the sk/fix-mots-export branch September 2, 2021 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AssertionError when dumping a dataset after I uploaded a MOTS annotation.
3 participants