issue_comments

hey @shoyer, @fmaussion. test decorator added with tutorial dataset unit test. i think this is good to go.

Hey @shoyer, if I have time later I will try to address the unit test issue. If I'm too late, feel free to pull this in the meantime.

ok, this should be good to go

Ok, sounds good. shutil was no longer needed, but the md5 checksum was implemented as private. So I think all is well.

On Tue, May 2, 2017 at 2:32 PM, Stephan Hoyer notifications@github.com wrote:

Thanks for taking this on @gidden https://github.com/gidden.

Also not sure how best to import shutil. I followed the current pattern for os, but am not sure why that pattern exists in this file.

I think this exists to work around the fact that IPython shows everything that isn't private in auto-complete (ipython/ipykernel#129 https://github.com/ipython/ipykernel/issues/129). But we should probably switch back to normal imports in this module -- this is really more of an IPython issue.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/1393#issuecomment-298737305, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVAEW5GuD6D_Y4ANjqM5AnK54h5BRZtks5r14TCgaJpZM4NOCuy .

Successful locally with pydata/xarray-data#9:

In [3]: xr.tutorial.load_dataset('air_temperature', github_url='https://github.com/gidden/xarray-data', branch='md5') Out[3]: <xarray.Dataset> Dimensions: (lat: 25, lon: 53, time: 2920) Coordinates: * lat (lat) float32 75.0 72.5 70.0 67.5 65.0 62.5 60.0 57.5 55.0 52.5 ... * lon (lon) float32 200.0 202.5 205.0 207.5 210.0 212.5 215.0 217.5 ... * time (time) datetime64[ns] 2013-01-01 2013-01-01T06:00:00 ... Data variables: air (time, lat, lon) float64 241.2 242.5 243.5 244.0 244.1 243.9 ... Attributes: Conventions: COARDS title: 4x daily NMC reanalysis (1948) description: Data is from NMC initialized reanalysis\n(4x/day). These a... platform: Model references: http://www.esrl.noaa.gov/psd/data/gridded/data.ncep.reanaly...

It seems like urlretrieve is coming directly from urllib. It seems in both Python 2 and 3 ContentTooShort error should be thrown and isnt. Perhaps checksums are the right way to go.

On Tue, May 2, 2017 at 7:14 PM, Fabien Maussion notifications@github.com wrote:

@fmaussion commented on this pull request.

In xarray/tutorial.py https://github.com/pydata/xarray/pull/1393#discussion_r114372912:

     url = '/'.join((github_url, 'raw', 'master', fullname))

• _urlretrieve(url, localfile)
• _urlretrieve(url, tmpfile) +
• if not _os.path.exists(tmpfile):
• raise ValueError('File could not be downloaded, please try again') +
• _shutil.move(tmpfile, localfile)

I thought that the issue wasn't that the file isn't downloaded, but rather that it is incompletely downloaded? Will this new temporary step than solve this?

Is there a way to make urlretrieve more robust, with checksums or something?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/1393#pullrequestreview-35833283, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVAEVQ4rFwcddY-kEOYnaEaLHsxf_u9ks5r12RkgaJpZM4NOCuy .