Skip to content

[WIP] Add Utility function to read image from the file as imread is deprecated #10149

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[WIP] Add Utility function to read image from the file as imread is deprecated #10149

wants to merge 2 commits into from

Conversation

keyur9
Copy link

@keyur9 keyur9 commented Nov 16, 2017 •
edited
Loading

Reference Issues/PRs

Fixes #10147

What does this implement/fix? Explain your changes.

Copy the implementation of scipy.misc.imread as imread is deprecated suggested using PIL/pillow directly

Any other comments?

@keyur9 keyur9 changed the title Fixed Issue 10147: load_sample_images uses deprecated imread Fix Issue #10147: load_sample_images uses deprecated imread Nov 16, 2017
jnothman
jnothman reviewed Nov 16, 2017
View reviewed changes
Copy link
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

I think we should make _imread a function in sklearn.utils

@glemaitre
Copy link
Member

glemaitre commented Nov 16, 2017 •
edited
Loading

I agree with @jnothman and you can add a test by using one of the image from sklearn.datasets.images

@glemaitre glemaitre changed the title Fix Issue #10147: load_sample_images uses deprecated imread [WIP] load_sample_images uses deprecated imread Nov 16, 2017
@glemaitre
Copy link
Member

I put removed the issue number (it will happen automatically when merging). Could you give a better title to the PR

@codecov
Copy link

codecov bot commented Nov 18, 2017

Codecov Report

Merging #10149 into master will not change coverage.
The diff coverage is 50%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #10149   +/-   ##
=======================================
  Coverage   96.21%   96.21%           
=======================================
  Files         337      337           
  Lines       62891    62891           
=======================================
  Hits        60508    60508           
  Misses       2383     2383
Impacted Files Coverage Δ
sklearn/datasets/lfw.py 12.41% <0%> (ø) ⬆️
sklearn/datasets/tests/test_base.py 97.12% <100%> (ø) ⬆️
sklearn/datasets/base.py 85.5% <66.66%> (ø) ⬆️

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 f6163e7...6e952f1. Read the comment docs.

@codecov
Copy link

codecov bot commented Nov 18, 2017

Codecov Report

Merging #10149 into master will not change coverage.
The diff coverage is 50%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #10149   +/-   ##
=======================================
  Coverage   96.21%   96.21%           
=======================================
  Files         337      337           
  Lines       62891    62891           
=======================================
  Hits        60508    60508           
  Misses       2383     2383
Impacted Files Coverage Δ
sklearn/datasets/lfw.py 12.41% <0%> (ø) ⬆️
sklearn/datasets/tests/test_base.py 97.12% <100%> (ø) ⬆️
sklearn/datasets/base.py 85.5% <66.66%> (ø) ⬆️

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 f6163e7...6e952f1. Read the comment docs.

@keyur9 keyur9 changed the title [WIP] load_sample_images uses deprecated imread [WIP] Add Utility function to read image from the file as imread is deprecated Nov 18, 2017
@keyur9
Copy link
Author

keyur9 commented Nov 18, 2017

Thank you for the feedback @jnothman and @glemaitre. I've created a utility function and added a test.

@jnothman
Copy link
Member

You have test failures...

jnothman
jnothman reviewed Nov 20, 2017
View reviewed changes
sklearn/utils/image_read.py
converted according to `mode`, and the result is then flattened using
mode 'F'.
"""
im = Image.open(name)
Copy link
Member

Choose a reason for hiding this comment

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

You don't do anything here with flatten or mode.

@glemaitre
Copy link
Member

@keyur9 are you going to work on the fix?

@keyur9
Copy link
Author

keyur9 commented Dec 12, 2017

Hello @glemaitre , Apologize for losing track of this and not updating the PR. Sorry, to inform that I won't be able to work on this until the weekend. So, if anyone want to work on this, feel free to do that or else will get back to this on the weekend. Thank you,

@jnothman
Copy link
Member

jnothman commented Dec 12, 2017 via email

@jotasi jotasi mentioned this pull request Jan 8, 2018
Merged
@qinhanmin2014
Copy link
Member

Resolved in #10427 @keyur9 Thanks for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jnothman jnothman jnothman left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

load_sample_images uses deprecated imread
4 participants
@keyur9 @glemaitre @jnothman @qinhanmin2014