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] Modify double lq_path to gt_path in test_pipeline #862

Merged
merged 7 commits into from
Apr 28, 2022
Merged

[Fix] Modify double lq_path to gt_path in test_pipeline #862

merged 7 commits into from
Apr 28, 2022

Conversation

ryanxingql
Copy link
Contributor

@ryanxingql ryanxingql commented Apr 24, 2022

Some meta_keys in config files might should be ['lq_path', 'gt_path'] instead of ['lq_path', 'lq_path'].

PS Two ipynb files 1 and 2 should be re-run to fix the typos. I haven't done that yet.

@codecov
Copy link

codecov bot commented Apr 24, 2022

Codecov Report

Merging #862 (bc3e3c4) into master (a852622) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #862   +/-   ##
=======================================
  Coverage   83.12%   83.12%           
=======================================
  Files         220      220           
  Lines       12453    12453           
  Branches     2017     2017           
=======================================
  Hits        10351    10351           
  Misses       1785     1785           
  Partials      317      317           
Flag Coverage Δ
unittests 83.08% <ø> (ø)

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


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 a852622...bc3e3c4. Read the comment docs.

@wangruohui
Copy link
Member

I think this is not a typo. In test pipeline, we don't actually have the gt_path.

@Yshuo-Li
Copy link
Collaborator

I think this is not a typo. In test_pipeline, we don't actually have the gt_path.

This is a typo.

  1. If there is no gt_path, the test pipeline cannot load gt by LoadImageFromFile.
  2. In demo scripts, if the configure file doesn't contain demo_pipeline, test_pipeline will be used for inference. However, demo scripts will remove gt and gt_path from the pipeline.

Besides, the reason why this typo doesn't raise any error is that gt_path is not used in the follow-up processing.

@wangruohui wangruohui requested review from Yshuo-Li and removed request for ckkelvinchan April 28, 2022 03:25
@wangruohui wangruohui changed the title [Improve] Fix typos [Fix] Modify double lq_path to lq_path and gt_path in Apr 28, 2022
@wangruohui wangruohui changed the title [Fix] Modify double lq_path to lq_path and gt_path in [Fix] Modify double lq_path to lq_path and gt_path in test_pipeline Apr 28, 2022
@wangruohui wangruohui changed the title [Fix] Modify double lq_path to lq_path and gt_path in test_pipeline [Fix] Modify double lq_path to gt_path in test_pipeline Apr 28, 2022
@Yshuo-Li Yshuo-Li merged commit 1f1e9bc into open-mmlab:master Apr 28, 2022
Yshuo-Li pushed a commit to Yshuo-Li/mmediting that referenced this pull request Jul 15, 2022
…mlab#862)

* polish

* Use all frames in a sequence dir

Some sequences may start from a frame index other than 0.

* Polish

* remove , to fix lint

* Add docstring for `start_idx`

* Fix typos

Co-authored-by: wangruohui <12756472+wangruohui@users.noreply.github.com>
Yshuo-Li pushed a commit to Yshuo-Li/mmediting that referenced this pull request Jul 15, 2022
…mlab#862)

* polish

* Use all frames in a sequence dir

Some sequences may start from a frame index other than 0.

* Polish

* remove , to fix lint

* Add docstring for `start_idx`

* Fix typos

Co-authored-by: wangruohui <12756472+wangruohui@users.noreply.github.com>
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.

None yet

3 participants