8000 failed doctests when PIL is not available · Issue #333 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

failed doctests when PIL is not available #333

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
fabianp opened this issue Sep 2, 2011 · 20 comments
Closed

failed doctests when PIL is not available #333

fabianp opened this issue Sep 2, 2011 · 20 comments
Assignees
@ogrisel
Milestone
0.10

Comments

@fabianp
Copy link
Member
fabianp commented Sep 2, 2011

here is the backtrace:

FAIL: Doctest: scikits.learn.datasets.base.load_sample_image
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fabian/lib/python2.7/doctest.py", line 2153, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for scikits.learn.datasets.base.load_sample_image
  File "/home/fabian/dev/scikit-learn/scikits/learn/datasets/base.py", line 403, in load_sample_image

----------------------------------------------------------------------
File "/home/fabian/dev/scikit-learn/scikits/learn/datasets/base.py", line 406, in scikits.learn.datasets.base.load_sample_image
Failed example:
    china = load_sample_image('china.jpg')
Exception raised:
    Traceback (most recent call last):
      File "/home/fabian/lib/python2.7/doctest.py", line 1248, in __run
        compileflags, 1) in test.globs
      File "<doctest scikits.learn.datasets.base.load_sample_image[0]>", line 1, in <module>
        china = load_sample_image('china.jpg')
      File "/home/fabian/dev/scikit-learn/scikits/learn/datasets/base.py", line 418, in load_sample_image
        images = load_sample_images()
      File "/home/fabian/dev/scikit-learn/scikits/learn/datasets/base.py", line 389, in load_sample_images
        raise ImportError("The Python Imaging Library (PIL)"
    ImportError: The Python Imaging Library (PIL)is required to load data from jpeg files
@ghost ghost assigned ogrisel Sep 2, 2011
@ogrisel
Copy link
Member
ogrisel commented Sep 2, 2011

Good catch. We need centralize the PIL related stuff somewhere and use it to disable tests on dependent methods.

@fabianp
Copy link
Member Author
fabianp commented Sep 14, 2011

Was this fixed ?

@ogrisel
Copy link
Member
ogrisel commented Sep 14, 2011

I don't think so, I completely forgot about it. I won't have time today. Maybe tomorrow, unless someone else does it first.

@fabianp
Copy link
Member Author
fabianp commented Sep 19, 2011

I had to comment the doctest for the release, but this still should be fixed

@amueller
Copy link
Member

How about using PNG instead of jpeg and use pylab.imread?
Then we would never have to rely on PIL.

@amueller
Copy link
Member
amueller commented Jan 8, 2012

for the record: pylab is not an official requirement of sklearn, so no!

@amueller
Copy link
Member
amueller commented Jan 8, 2012

as an afterthought: even if we sort out the pil issue, this will crash when pylab is unavailable....

@ogrisel: Where is pil used except in this file? Or do you mean to centralize for datasets.base and datasets.test.base ? The issue of not being able to skip doctests (see issue #342) would still cause some headache.

My suggestion would be to either add skiptests as I did in #526 or reference an example as @fabianp suggested.

@ogrisel
Copy link
Member
ogrisel commented Jan 8, 2012

Yes this code is almost duplicated in datasets.base and datasets.lfw. git grep imread to check.

@fabianp
Copy link
Member Author
fabianp commented Jan 9, 2012

@ogrisel : are you working on this or can we +SKIP it?. I'd really like to fix it before the release.

Same comment for issue #358

@ogrisel
Copy link
Member
ogrisel commented Jan 9, 2012

What's weird is that I did install a build of scikit-learn in virtualenv with --no-site-packages some time ago and did not experience the issue when running make. Need to retry on a recent build to check.

@amueller
Copy link
Member
amueller commented Jan 9, 2012

@fabianp I am +1 for SKIP (for both). What are the release plans btw?

@fabianp
Copy link
Member Author
fabianp commented Jan 9, 2012

I hope to have fixes most of the issues by the end of the week, then we can think of branching and release.

@fabianp
Copy link
Member Author
fabianp commented Jan 9, 2012

My priority right now is fixing the sklearn.test() failures.

@amueller
Copy link
Member
amueller commented Jan 9, 2012

If I can help you, just let me know.
I noticed there are some open issues for 0.10.
Do you think it would be helpful if I look into #7, #91 and/or #339?

The milestone 0.11 is due in 3 month. Is this already decided (just curious)?

By the way, I would really like your opinion on the SVM restructuring. Are you very busy at the moment?

@amueller
Copy link
Member
amueller commented Jan 9, 2012

My priority right now is fixing the sklearn.test() failures.

I didn't think there where any left... do you mean the warnings or the inplace import?

hmmm... I get some "AttributeError _" errors that I don't understand in cleaning up... forgot about those....

@fabianp
Copy link
Member Author
fabianp commented Jan 9, 2012

Hi Andreas,

I'm OK with the SVM restructuring. I'd however postpone the merging of that for the next release instead of doing it in a rush.

It's really helpful if you can look into the issues you mentioned. Please ask if it's not clear, I realize reading them now that some of them are rather cryptic. All of them are important but not blocking so I moved them to milestone 0.11.

@amueller
Copy link
Member
amueller commented Jan 9, 2012

Taking time for the SVM restructuring seems sensible.
I tried to get as many small fixes in as I can before the release. I'll see if I can address any more.

@fabianp
Copy link
Member Author
fabianp commented Jan 9, 2012

Great. As for the milestone, I just set it for 0.10 + 3 months because that's what our release cycle has approximately been like, but there has been no formal discussion.

@fabianp
Copy link
Member Author
fabianp commented Jan 9, 2012

"Fixed"" in dfc7c5c

@fabianp fabianp closed this as completed Jan 9, 2012
@ogrisel
Copy link
Member
ogrisel commented Jan 9, 2012

Thanks @fabianp.

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

@ogrisel ogrisel

Labels
None yet
Projects
None yet
Milestone
0.10
Development

No branches or pull requests
3 participants
@ogrisel @fabianp @amueller
0