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

[Datumaro] Support relative paths #1715

Merged
merged 5 commits into from
Jun 17, 2020
Merged

[Datumaro] Support relative paths #1715

merged 5 commits into from
Jun 17, 2020

Conversation

zhiltsov-max
Copy link
Contributor

Motivation and context

Preparation for #1463

  • Adds support for relative image paths in Datumaro
    • DatasetItem.id's now represent a relative path (without an extension)
    • DatasetItem.image.path now contains a real or supposed local image path
    • Format-specific image ids are moved to attributes
  • CVAT behaviour is left the same

How has this been tested?

Unit tests.

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) 2020 Intel Corporation
#
# SPDX-License-Identifier: MIT

Copy link
Contributor

Codacy Here is an overview of what got changed by this pull request:

Issues
======
- Added 1
           

Complexity increasing per file
==============================
- datumaro/tests/test_yolo_format.py  2
         

Complexity decreasing per file
==============================
+ datumaro/datumaro/plugins/voc_format/converter.py  -2
+ datumaro/datumaro/plugins/tf_detection_api_format/extractor.py  -1
+ datumaro/datumaro/plugins/yolo_format/converter.py  -1
         

See the complete overview on Codacy

# NOTE: when path is like [data/]<subset_obj>/<image_name>
# drop everything but <image name>
# <image name> can be <a/b/c/filename.ext>, so no just basename()
path = osp.join(*parts[1:])
Copy link
Contributor

Choose a reason for hiding this comment

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

@coveralls
Copy link

coveralls commented Jun 15, 2020

Pull Request Test Coverage Report for Build 5768

  • 76 of 79 (96.2%) changed or added relevant lines in 19 files are covered.
  • 6 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.03%) to 65.77%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cvat/apps/dataset_manager/bindings.py 7 8 87.5%
datumaro/datumaro/plugins/image_dir.py 7 8 87.5%
datumaro/datumaro/plugins/yolo_format/extractor.py 16 17 94.12%
Files with Coverage Reduction New Missed Lines %
cvat/apps/engine/media_extractors.py 6 75.88%
Totals Coverage Status
Change from base Build 5743: -0.03%
Covered Lines: 10881
Relevant Lines: 16153

💛 - Coveralls

save_image(osp.join(save_dir, item.id + '.jpg'),
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the original images were PNG or any other format? Don't you have all these issues just because item.id doesn't the the original extension?

Copy link
Contributor Author

@zhiltsov-max zhiltsov-max Jun 17, 2020

Choose a reason for hiding this comment

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

all these issues

Which ones?

  • Relative paths? They have no relation to extensions and are not used in any dataset format.
  • Appending an extension in converters? Maybe, but the extension, as well as fill image path, is available in item.image.path. And an option to copy original images files instead of recoding them is a matter of further PRs, as well as keeping the original extension, where applicable.

Meanwhile, we have to add an extension if we have a video frame, or if we have only frame data and no image info.

@@ -311,7 +311,9 @@ def __call__(self, extractor, save_dir):

if self._save_images:
if item.has_image and item.image.has_data:
self._save_image(item, index=frame_id)
save_image(osp.join(self._images_dir,
Copy link
Contributor

Choose a reason for hiding this comment

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

again. We have a limitation here.

Copy link
Contributor

@nmanovic nmanovic left a comment

Choose a reason for hiding this comment

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

I believe at the moment Datumaro has a design whole with extensions for images. But let's address the issue in another PR.

@nmanovic nmanovic merged commit 5912bf0 into develop Jun 17, 2020
@bsekachev bsekachev deleted the zm/dm-relative-paths branch June 17, 2020 15:42
frndmg pushed a commit to signatrix/cvat that referenced this pull request Aug 5, 2020
* Support relative image paths in Datumaro

* Update bindings

* Fix merge

* linter
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