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

[Fix] Merge resize and sr_resize. #310

Merged
merged 3 commits into from
May 18, 2021
Merged

Conversation

Yshuo-Li
Copy link
Collaborator

No description provided.

@codecov
Copy link

codecov bot commented May 16, 2021

Codecov Report

Merging #310 (7a9ac78) into master (1465ae5) will increase coverage by 0.15%.
The diff coverage is 100.00%.

❗ Current head 7a9ac78 differs from pull request most recent head 2b07cec. Consider uploading reports for the commit 2b07cec to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #310      +/-   ##
==========================================
+ Coverage   80.74%   80.89%   +0.15%     
==========================================
  Files         163      164       +1     
  Lines        8147     8213      +66     
  Branches     1194     1194              
==========================================
+ Hits         6578     6644      +66     
  Misses       1425     1425              
  Partials      144      144              
Flag Coverage Δ
unittests 80.89% <100.00%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmedit/datasets/pipelines/random_down_sampling.py 89.13% <ø> (ø)
mmedit/datasets/pipelines/__init__.py 100.00% <100.00%> (ø)
mmedit/datasets/pipelines/augmentation.py 97.04% <100.00%> (+0.04%) ⬆️
mmedit/models/backbones/sr_backbones/ttsr_net.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1465ae5...2b07cec. Read the comment docs.

@Yshuo-Li Yshuo-Li requested a review from innerlee May 17, 2021 12:04
@@ -33,7 +33,7 @@ class Resize:
keys (list[str]): The images to be resized.
scale (float | Tuple[int]): If scale is Tuple(int), target spatial
size (h, w). Otherwise, target spatial size is scaled by input
size. If any of scale is -1, we will rescale short edge.
size.
Copy link
Contributor

Choose a reason for hiding this comment

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

Reason for the deletion?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The code doesn't support scale=-1
if isinstance(scale, float): if scale <= 0: raise ValueError(f'Invalid scale {scale}, must be positive.')

backend (str | None): The image resize backend type. Options are `cv2`,
`pillow`, `None`. If backend is None, the global imread_backend
specified by ``mmcv.use_backend()`` will be used.
Default: "cv2".
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, the default backend should be None. Otherwise you will change behavior of old codes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In fact, if backend=None, cv2 will be used in mmcv.resize

@innerlee innerlee merged commit a0eaf22 into open-mmlab:master May 18, 2021
@Yshuo-Li Yshuo-Li deleted the resize branch May 20, 2021 02:31
Yshuo-Li added a commit to Yshuo-Li/mmediting that referenced this pull request Jul 15, 2022
* [Fix] Merge resize and sr_resize.

* Fix

* Fix

Co-authored-by: liyinshuo <[email protected]>
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.

2 participants