issue_comments

thanks!

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.

I'd be slightly more comfortable if we had this being unit tested on Travis-CI, but that's somewhat tricky to setup because we don't wan to require network access for the test suite by default. You could do this by adding a flag like --run-network-tests in conftest.py, and then hooking it through in xarray/tests/__init__.py like --run-flaky. I would be awesome if you could take a look at that and set the Travis/Appveyor tests to test the tutorial, but isn't strictly required to merge this.

Thanks @gidden, this look great to me. Could you kindly add a brief note to the "Bug fixes" section of whats-new.rst?

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 .

Thanks for taking this on @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 (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.

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 .