Added validation of attrs before saving to netCDF files (#991)

* Added validation of attrs before saving to netCDF files

This allows us to give nice errors if users try to save a Dataset with
attr values that can't be written to a netCDF file.

Fixes #911

* Used ds.variables, improved docs and refactored tests

* Added decorator for tests to ensure netCDF4 is installed

* Removed unused collections.abc import

* Fixed minor issues with tests and updated whats new

Author: robintw

doc/whats-new.rst

@@ -39,6 +39,9 @@ Deprecations
 
 Enhancements
 ~~~~~~~~~~~~
+- Add checking of ``attr`` names and values when saving to netCDF, raising useful
+error messages if they are invalid. (:issue:`911`).
+By `Robin Wilson <https://github.com/robintw>`_.
 
 Bug fixes
 ~~~~~~~~~

xarray/backends/api.py

@@ -5,6 +5,9 @@
 from distutils.version import StrictVersion
 from glob import glob
 from io import BytesIO
+from numbers import Number
+
+import numpy as np
 
 from .. import backends, conventions
 from .common import ArrayWriter
@@ -79,6 +82,37 @@ def check_name(name):
         check_name(k)
 
 
+def _validate_attrs(dataset):
+    """`attrs` must have a string key and a value which is either: a number
+    a string, an ndarray or a list/tuple of numbers/strings.
+    """
+    def check_attr(name, value):
+        if isinstance(name, basestring):
+            if not name:
+                raise ValueError('Invalid name for attr: string must be length '
+                                 '1 or greater for serialization to netCDF '
+                                 'files')
+        else:
+            raise TypeError("Invalid name for attr: {} must be a string for "
+                            "serialization to netCDF files".format(name))
+
+        if not isinstance(value, (basestring, Number, np.ndarray, np.number,
+                                  list, tuple)):
+            raise TypeError('Invalid value for attr: {} must be a number '
+                            'string, ndarray or a list/tuple of numbers/strings '
+                            'for serialization to netCDF '
+                            'files'.format(value))
+
+    # Check attrs on the dataset itself
+    for k, v in dataset.attrs.items():
+        check_attr(k, v)
+
+    # Check attrs on each variable within the dataset
+    for variable in dataset.variables.values():
+        for k, v in variable.attrs.items():
+            check_attr(k, v)
+
+
 def open_dataset(filename_or_obj, group=None, decode_cf=True,
                  mask_and_scale=True, decode_times=True,
                  concat_characters=True, decode_coords=True, engine=None,
@@ -335,8 +369,9 @@ def to_netcdf(dataset, path=None, mode='w', format=None, group=None,
     elif engine is None:
         engine = _get_default_engine(path)
 
-    # validate Dataset keys and DataArray names
+    # validate Dataset keys, DataArray names, and attr keys/values
     _validate_dataset_names(dataset)
+    _validate_attrs(dataset)
 
     try:
         store_cls = WRITEABLE_STORES[engine]

xarray/test/test_backends.py

@@ -1070,3 +1070,105 @@ def test_extract_h5nc_encoding(self):
                           {'least_sigificant_digit': 2})
         with self.assertRaisesRegexp(ValueError, 'unexpected encoding'):
             _extract_nc4_encoding(var, raise_on_invalid=True)
+
+
+class MiscObject:
+    pass
+
+@requires_netCDF4
+class TestValidateAttrs(TestCase):
+    def test_validating_attrs(self):
+        def new_dataset():
+            return Dataset({'data': ('y', np.arange(10.0))})
+
+        def new_dataset_and_dataset_attrs():
+            ds = new_dataset()
+            return ds, ds.attrs
+
+        def new_dataset_and_data_attrs():
+            ds = new_dataset()
+            return ds, ds.data.attrs
+
+        def new_dataset_and_coord_attrs():
+            ds = new_dataset()
+            return ds, ds.coords['y'].attrs
+
+        for new_dataset_and_attrs in [new_dataset_and_dataset_attrs,
+                                      new_dataset_and_data_attrs,
+                                      new_dataset_and_coord_attrs]:
+            ds, attrs = new_dataset_and_attrs()
+
+            attrs[123] = 'test'
+            with self.assertRaisesRegexp(TypeError, 'Invalid name for attr'):
+                ds.to_netcdf('test.nc')
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs[MiscObject()] = 'test'
+            with self.assertRaisesRegexp(TypeError, 'Invalid name for attr'):
+                ds.to_netcdf('test.nc')
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs[''] = 'test'
+            with self.assertRaisesRegexp(ValueError, 'Invalid name for attr'):
+                ds.to_netcdf('test.nc')
+
+            # This one should work
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = 'test'
+            with create_tmp_file() as tmp_file:
+                ds.to_netcdf(tmp_file)
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = {'a': 5}
+            with self.assertRaisesRegexp(TypeError, 'Invalid value for attr'):
+                ds.to_netcdf('test.nc')
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = MiscObject()
+            with self.assertRaisesRegexp(TypeError, 'Invalid value for attr'):
+                ds.to_netcdf('test.nc')
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = 5
+            with create_tmp_file() as tmp_file:
+                ds.to_netcdf(tmp_file)
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = 3.14
+            with create_tmp_file() as tmp_file:
+                ds.to_netcdf(tmp_file)
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = [1, 2, 3, 4]
+            with create_tmp_file() as tmp_file:
+                ds.to_netcdf(tmp_file)
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = (1.9, 2.5)
+            with create_tmp_file() as tmp_file:
+                ds.to_netcdf(tmp_file)
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = np.arange(5)
+            with create_tmp_file() as tmp_file:
+                ds.to_netcdf(tmp_file)
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = np.arange(12).reshape(3, 4)
+            with create_tmp_file() as tmp_file:
+                ds.to_netcdf(tmp_file)
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = 'This is a string'
+            with create_tmp_file() as tmp_file:
+                ds.to_netcdf(tmp_file)
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = ''
+            with create_tmp_file() as tmp_file:
+                ds.to_netcdf(tmp_file)
+
+            ds, attrs = new_dataset_and_attrs()
+            attrs['test'] = np.arange(12).reshape(3, 4)
+            with create_tmp_file() as tmp_file:
+                ds.to_netcdf(tmp_file)