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

[WIP] Server tests. Dataset manager. #2943

Conversation

DmitriyOparin
Copy link
Contributor

@DmitriyOparin DmitriyOparin commented Mar 12, 2021

Server tests. Dataset manager.

Motivation and context

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

@DmitriyOparin DmitriyOparin changed the title Server tests. Dataset manager. [WIP] Server tests. Dataset manager. Mar 12, 2021
@DmitriyOparin DmitriyOparin requested a review from azhavoro as a code owner March 12, 2021 07:25
else:
upload_format_name = dump_format_name

url = f"/api/v1/tasks/{task_id}/annotations?format={upload_format_name}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest introducing a function for this code block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Created new function.

task_ann.init_from_db()
task_data = TaskData(task_ann.ir_data, Task.objects.get(pk=task_id))
extractor = CvatTaskDataExtractor(task_data)
data_from_task_after_upload = datumaro.components.project.Dataset.from_extractors(extractor)
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 better to import Dataset and use it directly instead of having datumaro.components.project.. Currently, it resides in datumaro.components.dataset

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Fixed import.

dump_format_name = dump_format.DISPLAY_NAME

# TODO skip failed formats
if dump_format_name in [
Copy link
Contributor

Choose a reason for hiding this comment

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

changed points values

Points in segmentation formats (CamVid, Segmentation mask, Mots) can't be stored exactly, because they need to be recognized after converting into masks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. These formats are not checked.

]:
continue

print("dump_format_name:", dump_format_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using self.subTest() instead. You can find examples below in the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Added it actions.

"Segmentation mask 1.1", # changed points values
"WiderFace 1.0", # issue #XXX
]:
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using self.skip() instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Added it actions.

# upload annotations
url_upload = f"/api/v1/tasks/{task_id}/annotations?format={upload_format_name}"
with open(file_zip_name_after_change, 'rb') as binary_file:
with self.assertRaises(ValueError) as context:
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using assertRaisesRegex

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Added it actions.

task_data = TaskData(task_ann.ir_data, Task.objects.get(pk=task_id))
extractor = CvatTaskDataExtractor(task_data)
data_from_task_after_upload = datumaro.components.project.Dataset.from_extractors(extractor)
compare_datasets(self, data_from_task_before_upload, data_from_task_after_upload)
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using require_images=True along with save_images=True in line 648 for dataset export to compare image data. This can affect the kind of exported format in several formats.

Copy link
Contributor Author

@DmitriyOparin DmitriyOparin Mar 14, 2021

Choose a reason for hiding this comment

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

Done. Added include_images actions.

task = self._create_task(tasks["main"], images)

# create annotations
if dump_format_name in ["MOT 1.1", "MOTS PNG 1.0", "PASCAL VOC 1.1", "Segmentation mask 1.1", "TFRecord 1.0", "YOLO 1.1", "ImageNet 1.0", "WiderFace 1.0", "VGGFace2 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.

Strive to keep line length under 80 characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

"action": "download",
}
file_zip_name = osp.join(osp.dirname(__file__), 'assets', f'{test_name}_{dump_format_name}.zip')
self._download_file(url, data, self.admin, file_zip_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to split and simplify tests as much as possible. For example, I think, we could split format tests and API tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved tests to other file.

compare_datasets(self, data_datumaro["dataset"], data_datumaro["annotations"])

@skip("Fail")
def test_api_v1_tasks_annotations_dump_and_upload_with_datumaro_many_jobs(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clarify, what is the problem here? A task with multiple jobs is a regular case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test of check CVAT for images is passed, but CVAT for video is failed due to #2923 and #2945 bugs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 72.26% when pulling 31c1f2b on DmitriyOparin:upstream/do/server_test_dataset_manager into ca97507 on openvinotoolkit:develop.

@nmanovic
Copy link
Contributor

nmanovic commented Apr 2, 2021

We have another PR for these tests: #3004

@nmanovic nmanovic closed this Apr 2, 2021
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.

4 participants