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

Bug fixes in read_image_to_tensor #97

Merged
merged 3 commits into from
Apr 21, 2021

Conversation

stereomatchingkiss
Copy link
Contributor

@stereomatchingkiss stereomatchingkiss commented Apr 21, 2021

  1. Select cpu if gpu not available
  2. Change BGR to RGB before inference, this will fix some bugs in Add a dataloader as ultralytics in detection pipeline #92
  3. Append sys path, so the notebook can open on windows

2. Change BGR to RGB before inference
3. Append sys path, so the notebook can open on windows
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CLAassistant
Copy link

CLAassistant commented Apr 21, 2021

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Apr 21, 2021

Codecov Report

Merging #97 (bfd0362) into master (dbe2d4d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #97   +/-   ##
=======================================
  Coverage   83.39%   83.39%           
=======================================
  Files           8        8           
  Lines         777      777           
=======================================
  Hits          648      648           
  Misses        129      129           
Flag Coverage Δ
unittests 83.39% <ø> (ø)

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 dbe2d4d...bfd0362. Read the comment docs.

@review-notebook-app
Copy link

review-notebook-app bot commented Apr 21, 2021

View / edit / reply to this conversation on ReviewNB

zhiqwang commented on 2021-04-21T04:49:49Z
----------------------------------------------------------------

Actually we assume that the notebooks are decoupled with the path dependence, in other words, we assume this notebook could be working in any path, so I don't recommend adding sys path dependency here. (I am plan to add these notebooks to colab or kaggle, with some python library added, like pip install -U yolort) And this is the reason why I add get_image_from_url here, the image are downloaded from the internet.

@review-notebook-app
Copy link

review-notebook-app bot commented Apr 21, 2021

View / edit / reply to this conversation on ReviewNB

zhiqwang commented on 2021-04-21T04:49:50Z
----------------------------------------------------------------

Hi @stereomatchingkiss , I think that you could move the BGR2RGB operation to read_image_to_tensor directly.

https://github.com/zhiqwang/yolov5-rt-stack/blob/dbe2d4d1dd320be34d4c904e6cf6582ea91e868f/yolort/utils/image_utils.py#L167-L183

… with the path dependence

2. Remove color conversion because it perform in the function read_image_to_tensor
@stereomatchingkiss
Copy link
Contributor Author

Thanks for your helps, will fix

View / edit / reply to this conversation on ReviewNB

zhiqwang commented on 2021-04-21T04:49:50Z

Hi @stereomatchingkiss , I think that you could move the BGR2RGB operation to read_image_to_tensor directly.

https://github.com/zhiqwang/yolov5-rt-stack/blob/dbe2d4d1dd320be34d4c904e6cf6582ea91e868f/yolort/utils/image_utils.py#L167-L183

Thanks for your help, fixed

Copy link
Owner

@zhiqwang zhiqwang left a comment

Choose a reason for hiding this comment

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

LGTM!

@zhiqwang
Copy link
Owner

zhiqwang commented Apr 21, 2021

Hi @stereomatchingkiss , Thanks for your feedbacks and contributions here, welcome to join yolort community.

Besides, it would be more better if you could sign the CLA at CLA assistant check .

@zhiqwang zhiqwang changed the title Small update Bug fixes in read_image_to_tensor Apr 21, 2021
@zhiqwang zhiqwang merged commit f4bb74c into zhiqwang:master Apr 21, 2021
@zhiqwang zhiqwang added bug / fix Something isn't working and removed bugfix labels Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug / fix Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants