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 closing upon download completion in fetcher #227

Merged
merged 3 commits into from
Aug 23, 2013

Conversation

samuelstjean
Copy link
Contributor

The fileobject was not close originally and the md5 was checking a stil opened object, still always raising an error message on the first download. Now the object is closed before checking for the md5 and should work as intended.

@arokem
Copy link
Contributor

arokem commented Aug 2, 2013

Since that same sequence repeats so many times, wouldn't you rather make a helper function out of that?

def _get_file_data(fname, opener):
    data = open(fname, 'wb')
    data.write(opener.read())
    data.close()

Or something along these lines

@MrBago
Copy link
Contributor

MrBago commented Aug 2, 2013

How about something like:

from shutil import copyfileobj

def _get_file_data(fname, url):
    with openurl(url) as opener:
        with open(fname, 'wb') as data:
            copyfileobj(opener, data)

You got to watch out for a few things with something like this. You want to make sure that you close both opener and data, even when there is an error. The with statement will take care of that. Also write(file.read()) reads the whole thing into memory and then writes the whole thing in one go, that can be a problem with large files it's better to chunk it out. copyfileobj takes care of that.

@samuelstjean
Copy link
Contributor Author

Seems like with openurl(url) as opener: is not valid. I get something like

//home/stjs2902/git/dipy/dipy/data/fetcher.py in _get_file_data(fname, url)
21
22 def _get_file_data(fname, url):
---> 23 with urlopen(url) as opener:
24 with open(fname, 'wb') as data:
25 copyfileobj(opener.read(), data)

AttributeError: addinfourl instance has no attribute 'exit'

Is the with statement essential? We could close it by hand, since it seems that it's missing something it doesn't like.

@MrBago
Copy link
Contributor

MrBago commented Aug 5, 2013

Good catch, apparently urllib doesn't support the with statement directly until python 3, so we'll need to use contextlib.

http://stackoverflow.com/questions/1522636/should-i-call-close-after-urllib-urlopen

Garyfallidis added a commit that referenced this pull request Aug 23, 2013
Fix closing upon download completion in fetcher
@Garyfallidis Garyfallidis merged commit aa39117 into dipy:master Aug 23, 2013
@samuelstjean samuelstjean deleted the fix_fetcher branch August 23, 2013 13:54
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.

4 participants