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

[GSoC2024] Fix missing related images 3d dataset exports #7699

Closed
wants to merge 15 commits into from

Conversation

bargav25
Copy link

@bargav25 bargav25 commented Mar 28, 2024

Issue Addressed

I resolved issue #7375, which pertained to the omission of related images from archives when exporting tasks in the Datumaro 3D format. This problem was pinpointed to the Datumaro framework's specific handling of the media parameter during the export process. When this parameter is specified, related_images were not being considered, leading to their absence in the export results and, consequently, to incomplete dataset exports.

To overcome this limitation and guarantee the inclusion of related images in the export outcomes, I have integrated the related images into the export process via the PointCloud object.

Solution Implemented

Further analysis revealed that the PointCloud constructor is specifically designed to accept an extra_images argument, aimed at encapsulating related images. However, simply passing the related images as string paths directly was inadequate because the extra_images argument requires a list of objects that possess a .path attribute—namely,Image objects. This mismatch between the expected input format and the provided string paths called for a crucial modification in the handling of related images.

To address this, I converted the related images, represented by string paths (dm_image[1]), into Image objects prior to passing them to the PointCloud constructor. This change ensures that the extra_images parameter is supplied with data in the correct format, thus enabling the successful inclusion of related images in the dataset export process.

Testing Methodology

To verify the effectiveness of this solution, I conducted a test using the test_canvas3d.zip archive found in the cvat/apps/engine/tests/assets directory of the CVAT repository. After creating a task with this archive and exporting it in the Datumaro 3D format, I confirmed that the related images, previously missing, were present in the resulting archive. This outcome affirms the resolution of the issue.

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • I have linked related issues (see GitHub docs)
  • I have increased versions of npm packages if it is necessary
    (cvat-canvas,
    cvat-core,
    cvat-data and
    cvat-ui)

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.

@zhiltsov-max
Copy link
Contributor

Hi, thank you for the PR! Please add tests and make sure the paths are relative to the dataset root (not absolute). You can borrow some bits from this PR.

@zhiltsov-max zhiltsov-max mentioned this pull request Apr 2, 2024
7 tasks
@zhiltsov-max zhiltsov-max changed the title [GSoC2024] Fix Proposal for Issue #7375 : (Related images are not included when exporting 3D datasets) [GSoC2024] Fix missing related images 3d dataset exports Apr 3, 2024
zhiltsov-max added a commit that referenced this pull request Apr 5, 2024
<!-- Raise an issue to propose your change
(https://github.com/opencv/cvat/issues).
It helps to avoid duplication of efforts from multiple independent
contributors.
Discuss your ideas with maintainers to be sure that changes will be
approved and merged.
Read the [Contribution
guide](https://opencv.github.io/cvat/docs/contributing/). -->

<!-- Provide a general summary of your changes in the Title above -->

### Motivation and context
<!-- Why is this change required? What problem does it solve? If it
fixes an open
issue, please link to the issue here. Describe your changes in detail,
add
screenshots. -->

Closes #7703
Related: #7258
Related: #7125
Related: #7699

Changes:
cvat-ai/datumaro@8a14a99...82982b1

- Fixed WiderFace dataset example
- Fixed export without images in Datumaro format - no empty `media` and
`point_cloud` fields should be in the results

### How has this been tested?
<!-- Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc. -->

### Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply.
If an item isn't applicable for some reason, then ~~explicitly
strikethrough~~ the whole
line. If you don't do that, GitHub will show incorrect progress for the
pull request.
If you're unsure about any of these, don't hesitate to ask. We're here
to help! -->
- [ ] I submit my changes into the `develop` branch
- [ ] I have created a changelog fragment <!-- see top comment in
CHANGELOG.md -->
- [ ] I have updated the documentation accordingly
- [ ] I have added tests to cover my changes
- [ ] I have linked related issues (see [GitHub docs](

https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword))
- [ ] I have increased versions of npm packages if it is necessary

([cvat-canvas](https://github.com/opencv/cvat/tree/develop/cvat-canvas#versioning),

[cvat-core](https://github.com/opencv/cvat/tree/develop/cvat-core#versioning),

[cvat-data](https://github.com/opencv/cvat/tree/develop/cvat-data#versioning)
and

[cvat-ui](https://github.com/opencv/cvat/tree/develop/cvat-ui#versioning))

### License

- [ ] I submit _my code changes_ under the same [MIT License](
https://github.com/opencv/cvat/blob/develop/LICENSE) that covers the
project.
  Feel free to contact the maintainers if that's a concern.
@zhiltsov-max
Copy link
Contributor

Hi, any updates on this PR?

@bargav25
Copy link
Author

will add tests by EOD

SpecLad and others added 6 commits April 10, 2024 14:33
<!-- Raise an issue to propose your change
(https://github.com/opencv/cvat/issues).
It helps to avoid duplication of efforts from multiple independent
contributors.
Discuss your ideas with maintainers to be sure that changes will be
approved and merged.
Read the [Contribution
guide](https://opencv.github.io/cvat/docs/contributing/). -->

<!-- Provide a general summary of your changes in the Title above -->

### Motivation and context
<!-- Why is this change required? What problem does it solve? If it
fixes an open
issue, please link to the issue here. Describe your changes in detail,
add
screenshots. -->

### How has this been tested?
<!-- Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc. -->

### Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply.
If an item isn't applicable for some reason, then ~~explicitly
strikethrough~~ the whole
line. If you don't do that, GitHub will show incorrect progress for the
pull request.
If you're unsure about any of these, don't hesitate to ask. We're here
to help! -->
- [ ] I submit my changes into the `develop` branch
- [ ] I have created a changelog fragment <!-- see top comment in
CHANGELOG.md -->
- [ ] I have updated the documentation accordingly
- [ ] I have added tests to cover my changes
- [ ] I have linked related issues (see [GitHub docs](

https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword))
- [ ] I have increased versions of npm packages if it is necessary

([cvat-canvas](https://github.com/opencv/cvat/tree/develop/cvat-canvas#versioning),

[cvat-core](https://github.com/opencv/cvat/tree/develop/cvat-core#versioning),

[cvat-data](https://github.com/opencv/cvat/tree/develop/cvat-data#versioning)
and

[cvat-ui](https://github.com/opencv/cvat/tree/develop/cvat-ui#versioning))

### License

- [ ] I submit _my code changes_ under the same [MIT License](
https://github.com/opencv/cvat/blob/develop/LICENSE) that covers the
project.
  Feel free to contact the maintainers if that's a concern.

---------

Co-authored-by: Maxim Zhiltsov <zhiltsov.max35@gmail.com>
cvat-ai#7697)

<!-- Raise an issue to propose your change
(https://github.com/opencv/cvat/issues).
It helps to avoid duplication of efforts from multiple independent
contributors.
Discuss your ideas with maintainers to be sure that changes will be
approved and merged.
Read the [Contribution
guide](https://opencv.github.io/cvat/docs/contributing/). -->

<!-- Provide a general summary of your changes in the Title above -->

### Motivation and context
<!-- Why is this change required? What problem does it solve? If it
fixes an open
issue, please link to the issue here. Describe your changes in detail,
add
screenshots. -->

Allows to download annotated video frames with custom extensions.
Previously, it was only available in .png format. Resolves cvat-ai#3867

### Checklist
<!-- Go over all the following points, and put an `x` in all the boxes
that apply.
If an item isn't applicable for some reason, then ~~explicitly
strikethrough~~ the whole
line. If you don't do that, GitHub will show incorrect progress for the
pull request.
If you're unsure about any of these, don't hesitate to ask. We're here
to help! -->
- [x] I submit my changes into the `develop` branch
- [ ] I have created a changelog fragment <!-- see top comment in
CHANGELOG.md -->
- [ ] I have updated the documentation accordingly
- [ ] I have added tests to cover my changes
- [x] I have linked related issues (see [GitHub docs](

https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword))
- [x] I have increased versions of npm packages if it is necessary

([cvat-canvas](https://github.com/opencv/cvat/tree/develop/cvat-canvas#versioning),

[cvat-core](https://github.com/opencv/cvat/tree/develop/cvat-core#versioning),

[cvat-data](https://github.com/opencv/cvat/tree/develop/cvat-data#versioning)
and

[cvat-ui](https://github.com/opencv/cvat/tree/develop/cvat-ui#versioning))

### License

- [x] I submit _my code changes_ under the same [MIT License](
https://github.com/opencv/cvat/blob/develop/LICENSE) that covers the
project.
  Feel free to contact the maintainers if that's a concern.

---------

Co-authored-by: Maxim Zhiltsov <zhiltsov.max35@gmail.com>
@bargav25
Copy link
Author

Added in cvat/apps/engine/tests/test_rest_api_3D.py . Pls take a look

cvat/apps/dataset_manager/bindings.py Outdated Show resolved Hide resolved
cvat/apps/dataset_manager/bindings.py Outdated Show resolved Hide resolved
cvat/apps/engine/tests/test_rest_api_3D.py Outdated Show resolved Hide resolved
@zhiltsov-max
Copy link
Contributor

zhiltsov-max commented Apr 11, 2024

  • Please fix the failing tests (probably, some checked paths need to be updated - the same tests had been failing before the suggestions were merged)
  • Please add a changelog entry following the guide.

@bargav25
Copy link
Author

I can fix the Datumaro issue. However, the Sly format is not exporting related_images, which is why the error occurs, since we set related_files = True this time.

@zhiltsov-max
Copy link
Contributor

zhiltsov-max commented Apr 12, 2024

I can fix the Datumaro issue. However, the Sly format is not exporting related_images, which is why the error occurs, since we set related_files = True this time.

What do you mean by this? I can clearly see related images in the archive, but they have slightly different paths.

Co-authored-by: Maxim Zhiltsov <zhiltsov.max35@gmail.com>
@bargav25
Copy link
Author

Got it. Will change asap

@bargav25
Copy link
Author

Done. Pls take a look

@@ -761,5 +791,5 @@ def test_api_v2_export_dataset(self):
with open(file_name, "wb") as f:
f.write(content.getvalue())
self.assertEqual(osp.exists(file_name), edata['file_exists'])
self._check_dump_content(content, task_ann_prev.data, format_name,related_files=False)
self._check_dump_content(content, task_ann_prev.data, format_name, related_files=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self._check_dump_content(content, task_ann_prev.data, format_name, related_files=True)
if edata['file_exists']:
self._check_dump_content(content, task_ann_prev.data, format_name, related_files=True)

It seems there is an error in the test.

]
point_cloud_files_present = any(osp.exists(f) for f in point_cloud_files)
if related_files and point_cloud_files_present:
checking_files.extend([
Copy link
Contributor

Choose a reason for hiding this comment

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

This code doesn't seem to be executed

@bsekachev
Copy link
Member

Hi @bargav-bunny

Do you have any plans regarding the pull request?

@bsekachev
Copy link
Member

Please, let us know if you are ready to finish the pull request. I will reopen it.

@bsekachev bsekachev closed this Apr 25, 2024
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.

7 participants