issue_comments

Hey @fmaussion, is this ready for me to try out again? I wanted to let you and @shoyer iterate first.

Very welcome, @shoyer!

On Thu, May 25, 2017 at 4:06 AM, Stephan Hoyer notifications@github.com wrote:

Thanks @gidden https://github.com/gidden!

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

TIL, thanks!

On Wed, May 24, 2017 at 10:34 AM, Stephan Hoyer notifications@github.com wrote:

@shoyer commented on this pull request.

In xarray/tests/init.py https://github.com/pydata/xarray/pull/1423#discussion_r118193430:

         yield

• assert len(w) > 0
• assert any(message in str(wi.message) for wi in w)
• assert len(w) > 0

Yes, this was intentional. Python with blocks don't define scopes for variables, so w is still defined outside the block. In this case, the with block only indicates where warnings should be caught.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/1423#discussion_r118193430, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVAESisUh7dTNsp3l4OryHoCGuJPb2gks5r8-uugaJpZM4Nkq_Z .

sounds good!

On Wed, May 24, 2017 at 10:01 AM, Stephan Hoyer notifications@github.com wrote:

@gidden https://github.com/gidden this isn't you -- pytest broke us (I'll merge #1423 https://github.com/pydata/xarray/pull/1423 with a fix shortly)

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

hey @shoyer, this is failing one each of a dataset and dataarray test. I am not sure why, could you please assist? thanks!

Sounds good!

Note that the above tiff was made with something like the following code:

``` res = 0.5 nlat = 180 nlon = 360 left_lon = -180 upper_lat = 90 crs = 'epsg:4326'

profile = { 'affine': rio.Affine(res, 0.0, left_lon, 0.0, -res, upper_lat), 'dtype': 'int32', 'height': int(nlat / res), 'width': int(nlon / res), 'nodata': '-1', 'crs': crs, } ```

Hey @fmaussion, I guess my question here is as follows: if a raster was generated by rasterio (thus using a dictionary representation of the CRS object even if a string is provided), should xarrary then fail to write to netcdf with that dataset?

If so, then that means that all users will have to do

ds = open_rasterio('file.tiff') ds.attrs['crs'] = ds.attrs['crs'].to_string() ds.to_netcdf('file.nc')

Would it be better to use the string representation of the CRS internally after reading in? I think this would solve @shoyer's concern about onboarding non-primitive datatypes.

As an aside, even my above suggestion did not work.. I'm not sure why.

``` In [33]: ds = xr.open_rasterio('./isimip_centroid_0_5.tiff')

In [34]: ds Out[34]: <xarray.Dataset> Dimensions: (band: 1, x: 720, y: 360) Coordinates: * y (y) float64 90.0 89.5 89.0 88.5 88.0 87.5 87.0 86.5 86.0 85.5 ... * x (x) float64 -180.0 -179.5 -179.0 -178.5 -178.0 -177.5 -177.0 ... * band (band) int64 1 Data variables: raster (band, y, x) int32 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 ... Attributes: crs: CRS({'init': u'epsg:4326'})

In [35]: ds.attrs['crs'] = ds.attrs['crs'].to_string()

In [36]: ds Out[36]: <xarray.Dataset> Dimensions: (band: 1, x: 720, y: 360) Coordinates: * y (y) float64 90.0 89.5 89.0 88.5 88.0 87.5 87.0 86.5 86.0 85.5 ... * x (x) float64 -180.0 -179.5 -179.0 -178.5 -178.0 -177.5 -177.0 ... * band (band) int64 1 Data variables: raster (band, y, x) int32 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 ... Attributes: crs: +init=epsg:4326

In [37]: ds.to_netcdf('test.nc')

TypeError Traceback (most recent call last) <ipython-input-37-d7687fe1d488> in <module>() ----> 1 ds.to_netcdf('test.nc')

/home/gidden/.local/lib/python2.7/site-packages/xarray-0.9.5_34_g48c7268-py2.7.egg/xarray/core/dataset.pyc in to_netcdf(self, path, mode, format, group, engine, encoding, unlimited_dims) 975 return to_netcdf(self, path, mode, format=format, group=group, 976 engine=engine, encoding=encoding, --> 977 unlimited_dims=unlimited_dims) 978 979 def unicode(self):

/home/gidden/.local/lib/python2.7/site-packages/xarray-0.9.5_34_g48c7268-py2.7.egg/xarray/backends/api.pyc in to_netcdf(dataset, path_or_file, mode, format, group, engine, writer, encoding, unlimited_dims) 588 # validate Dataset keys, DataArray names, and attr keys/values 589 _validate_dataset_names(dataset) --> 590 _validate_attrs(dataset) 591 592 try:

/home/gidden/.local/lib/python2.7/site-packages/xarray-0.9.5_34_g48c7268-py2.7.egg/xarray/backends/api.pyc in _validate_attrs(dataset) 119 for variable in dataset.variables.values(): 120 for k, v in variable.attrs.items(): --> 121 check_attr(k, v) 122 123

/home/gidden/.local/lib/python2.7/site-packages/xarray-0.9.5_34_g48c7268-py2.7.egg/xarray/backends/api.pyc in check_attr(name, value) 110 'string, ndarray or a list/tuple of ' 111 'numbers/strings for serialization to netCDF ' --> 112 'files'.format(value)) 113 114 # Check attrs on the dataset itself

TypeError: Invalid value for attr: CRS({'init': u'epsg:4326'}) must be a number string, ndarray or a list/tuple of numbers/strings for serialization to netCDF files ```

Hey @fmaussion, should we also do any attribute checking, or should we let the user fails as follows? I recently was testing this out and made a *.tiff with a shapefile as read from fiona. I got the following error:

``` In [12]: ds Out[12]: <xarray.Dataset> Dimensions: (band: 1, x: 720, y: 360) Coordinates: * y (y) float64 90.0 89.5 89.0 88.5 88.0 87.5 87.0 86.5 86.0 85.5 ... * x (x) float64 -180.0 -179.5 -179.0 -178.5 -178.0 -177.5 -177.0 ... * band (band) int64 1 Data variables: raster (band, y, x) int32 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 ... Attributes: crs: CRS({'init': u'epsg:4326'})

In [13]: ds.to_netcdf('test.nc')

TypeError Traceback (most recent call last) <ipython-input-12-d7687fe1d488> in <module>() ----> 1 ds.to_netcdf('test.nc')

/home/gidden/.local/lib/python2.7/site-packages/xarray-0.9.5_34_g48c7268-py2.7.egg/xarray/core/dataset.pyc in to_netcdf(self, path, mode, format, group, engine, encoding, unlimited_dims) 975 return to_netcdf(self, path, mode, format=format, group=group, 976 engine=engine, encoding=encoding, --> 977 unlimited_dims=unlimited_dims) 978 979 def unicode(self):

/home/gidden/.local/lib/python2.7/site-packages/xarray-0.9.5_34_g48c7268-py2.7.egg/xarray/backends/api.pyc in to_netcdf(dataset, path_or_file, mode, format, group, engine, writer, encoding, unlimited_dims) 588 # validate Dataset keys, DataArray names, and attr keys/values 589 _validate_dataset_names(dataset) --> 590 _validate_attrs(dataset) 591 592 try:

/home/gidden/.local/lib/python2.7/site-packages/xarray-0.9.5_34_g48c7268-py2.7.egg/xarray/backends/api.pyc in _validate_attrs(dataset) 114 # Check attrs on the dataset itself 115 for k, v in dataset.attrs.items(): --> 116 check_attr(k, v) 117 118 # Check attrs on each variable within the dataset

/home/gidden/.local/lib/python2.7/site-packages/xarray-0.9.5_34_g48c7268-py2.7.egg/xarray/backends/api.pyc in check_attr(name, value) 110 'string, ndarray or a list/tuple of ' 111 'numbers/strings for serialization to netCDF ' --> 112 'files'.format(value)) 113 114 # Check attrs on the dataset itself

TypeError: Invalid value for attr: CRS({'init': u'epsg:4326'}) must be a number string, ndarray or a list/tuple of numbers/strings for serialization to netCDF files ```

And it seems like this happens whether I use a dictionary originally or just a string.

Perhaps this is a slightly larger issue with cleaning the CRS object such that it is netcdf compatible? Does this mean just a

crs = ' '.join('+{}={}'.format(k, v) for k, v in crs.items()) '?

@byersiiasa indeed I am, in fact I'm reviewing #1260 which overtook this one =)

Hey @fmaussion, I personally think some lat/lon support should be included here. I would lean toward 1 personally, but would be interested in what others think.

ok, should be cleaned up and ready to go @shoyer

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 .