rasterio backend: added nodatavals attribute #1740
Conversation
Connected with issue #1736
Fixes the type error with serialization: https://travis-ci.org/pydata/xarray/jobs/306107679
|
@fmaussion can you take a look? |
There was a problem hiding this comment.
Thanks! I only have a suggestion for the what's new entry, and also it would be good to add a simple test to ensure that we don't break this later on.
see e.g. https://github.com/pydata/xarray/blob/master/xarray/tests/test_backends.py#L2139-L2203
doc/whats-new.rst
Outdated
| @@ -20,6 +20,7 @@ v0.10.1 (unreleased) | |||
|
|
|||
| Enhancements | |||
| ~~~~~~~~~~~~ | |||
| - Added nodatavals attribute to dataset when using open_rasterio. | |||
There was a problem hiding this comment.
If you want you can credit yourself here. If you do so it would be good also to link to the original issue (see examples below) and to rename dataset do DataArray
xarray/tests/test_backends.py
Outdated
| @@ -1871,11 +1871,39 @@ def test_serialization(self): | |||
|
|
|||
| # Write it to a netcdf and read again (roundtrip) | |||
| with xr.open_rasterio(tmp_file) as rioda: | |||
| np.testing.assert_array_equal(rioda.attrs['nodatavals'], | |||
| [np.nan, np.nan, np.nan]) | |||
There was a problem hiding this comment.
Can you put this test in another test? For example in test_utm below
fmaussion
changed the title
|
@fmaussion This looks good to me, please merge at your leisure |
|
Indeed, it would be nice to refactor the tests to use pytest functions so we can use fixtures and the pytest.mark.parametrize to reduce duplication. |
Connected with issue #1736
git diff upstream/master **/*py | flake8 --diffwhats-new.rstfor all changes andapi.rstfor new API