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

[Feature] Add multi file backends to imread/imwrite. #1527

Merged
merged 12 commits into from
Dec 14, 2021

Conversation

274869388
Copy link
Contributor

Motivation

The motivation of the PR is to read image from different file backends and write image to different file backends but do not need to modify the existing interface.

It includes two parts:

  • Add FileClient to image io function to support different file backends.
  • Add unittest to tests/test_image/test_io.py.

Modification

  1. imread

    • Add a new parameter file_client_args to load which makes it support loading object from different file backends, but the parameter is optional
    • Add FileClient to read image bytes of different file backends and use imfrombytes to decode image.
    • Add tifffile backends to imfrombytes.
  2. imwrite

    • Add a new parameter file_client_args to load which makes it support loading object from different file backends, but the parameter is optional
    • Add cv2.imencode to encode image and use FileClient to write image bytes to different file backends.
    • Warn when image writing fails.
  3. test_io.py

    • Add MagicMock to test imread and imwrite.

BC-breaking (Optional)

No

Use cases (Optional)

import mmcv

disk_path = '/path/of/your/file'
img = mmcv.imread(disk_path)
mmcv.imwrite(img, disk_path)

s3_path = 's3://your_bucket/img.png'
img = mmcv.imread(s3_path)
img = mmcv.imread(s3_path, file_client_args={'backend': 'petrel'})
img = mmcv.imread(s3_path, file_client_args={'prefix': 's3'})
mmcv.imwrite(img, s3_path)
mmcv.imwrite(img, s3_path, file_client_args={'backend': 'petrel'})
mmcv.imwrite(img, s3_path, file_client_args={'prefix': 's3'})

http_path = 'http://path/of/your/file'
img = mmcv.imread(http_path)
img = mmcv.imread(http_path, file_client_args={'backend': 'http'})
img = mmcv.imread(http_path, file_client_args={'prefix': 'http'})
mmcv.imwrite(img, http_path)
...

Test

  • disk backend works as expected
  • petrel backend works as expected

Checklist

Before PR:

  • I have read and followed the workflow indicated in the CONTRIBUTING.md to create this PR.
  • Pre-commit or linting tools indicated in CONTRIBUTING.md are used to fix the potential lint issues.
  • Bug fixes are covered by unit tests, the case that causes the bug should be added in the unit tests.
  • New functionalities are covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  • The documentation has been modified accordingly, including docstring or example tutorials.

After PR:

  • If the modification has potential influence on downstream or other related projects, this PR should be tested with some of those projects, like MMDet or MMCls.
  • CLA has been signed and all committers have signed the CLA in this PR.

@CLAassistant
Copy link

CLAassistant commented Nov 26, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@teamwong111 teamwong111 left a comment

Choose a reason for hiding this comment

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

LGTM but with some slight doubt.

  1. Is it necessary to add some novel unittests for imwrite with other file backend?
  2. Shall we refactor test file which looks like not elegant now?
  3. Shall we impl exists function for all File backend before merging this PR?

mmcv/image/io.py Outdated Show resolved Hide resolved
mmcv/image/io.py Show resolved Hide resolved
mmcv/image/io.py Outdated Show resolved Hide resolved
@274869388
Copy link
Contributor Author

LGTM but with some slight doubt.

  1. Is it necessary to add some novel unittests for imwrite with other file backend?
  2. Shall we refactor test file which looks like not elegant now?
  3. Shall we impl exists function for all File backend before merging this PR?
  1. I think there is no need to test other file backends because they have been fully tested in test_fileio.py.
  2. I may not be ready to refactor the test file for now.
  3. We plan to impl the exists function for all file backend in the future,but not now.

@274869388 274869388 changed the title Imageio [Feature] Add multi file backends to imread/imwrite. Nov 29, 2021
Copy link
Contributor

@teamwong111 teamwong111 left a comment

Choose a reason for hiding this comment

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

LGTM

@zhouzaida
Copy link
Collaborator

Maybe we can also add multi-file backends support for the flowread and flowwrite

@MeowZheng
Copy link
Collaborator

Maybe we can also add multi-file backends support for the flowread and flowwrite

Of course, but I think we should create another pr for it. I will take this work very recently.

mmcv/image/io.py Outdated Show resolved Hide resolved
mmcv/image/io.py Outdated Show resolved Hide resolved
mmcv/image/io.py Outdated Show resolved Hide resolved
mmcv/image/io.py Outdated Show resolved Hide resolved
mmcv/image/io.py Outdated Show resolved Hide resolved
mmcv/image/io.py Outdated Show resolved Hide resolved
mmcv/image/io.py Outdated Show resolved Hide resolved
@ZwwWayne ZwwWayne merged commit 66bff13 into open-mmlab:master Dec 14, 2021
@274869388 274869388 deleted the imageio branch December 14, 2021 06:38
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.

6 participants