Support name tokens (#3399)

* Support name tokens

* Don't token check the CellMethod method

* Added minimal test coverage

* Add whatsnew bugfix entry

* add class constant

* review actions

docs/iris/src/whatsnew/contributions_2.3.0/bugfix_2019-Sep-16_cellmethod_tokens.txt

@@ -0,0 +1 @@
+* :class:`~iris.coords.CellMethod` will now only use valid `NetCDF name tokens <https://www.unidata.ucar.edu/software/netcdf/docs/netcdf_data_set_components.html#object_name>`_ to reference the coordinates involved in the statistical operation.

docs/iris/src/whatsnew/contributions_2.3.0/bugfix_2019-Sep-16_var_name_tokens.txt

@@ -0,0 +1 @@
+* :attr:`~iris.aux_factory.AuxCoordFactory.var_name`, :attr:`~iris.coords.CellMeasure.var_name`, :attr:`~iris.coords.Coord.var_name`, :attr:`~iris.coords.AuxCoord.var_name` and :attr:`~iris.cube.Cube.var_name` will now only use valid `NetCDF name tokens <https://www.unidata.ucar.edu/software/netcdf/docs/netcdf_data_set_components.html#object_name>`_ to reference the said NetCDF variable name. Note that, names with a leading inderscore are not permitted.

lib/iris/_cube_coord_common.py

@@ -1,4 +1,4 @@
-# (C) British Crown Copyright 2010 - 2018, Met Office
+# (C) British Crown Copyright 2010 - 2019, Met Office
 #
 # This file is part of Iris.
 #
@@ -19,15 +19,18 @@
 from six.moves import (filter, input, map, range, zip)  # noqa
 import six
 
-# TODO: Is this a mixin or a base class?
-
+import re
 import string
 
 import cf_units
 
 import iris.std_names
 
 
+# https://www.unidata.ucar.edu/software/netcdf/docs/netcdf_data_set_components.html#object_name
+_TOKEN_PARSE = re.compile(r'''^[a-zA-Z0-9][\w\.\+\-@]*$''')
+
+
 class LimitedAttributeDict(dict):
     _forbidden_keys = ('standard_name', 'long_name', 'units', 'bounds', 'axis',
                        'calendar', 'leap_month', 'leap_year', 'month_lengths',
@@ -84,17 +87,66 @@ def update(self, other, **kwargs):
 
 
 class CFVariableMixin(object):
-    def name(self, default='unknown'):
+
+    _DEFAULT_NAME = 'unknown'  # the name default string
+
+    @staticmethod
+    def token(name):
+        '''
+        Determine whether the provided name is a valid NetCDF name and thus
+        safe to represent a single parsable token.
+
+        Args:
+
+        * name:
+            The string name to verify
+
+        Returns:
+            The provided name if valid, otherwise None.
+
+        '''
+        if name is not None:
+            result = _TOKEN_PARSE.match(name)
+            name = result if result is None else name
+        return name
+
+    def name(self, default=None, token=False):
         """
         Returns a human-readable name.
 
         First it tries :attr:`standard_name`, then 'long_name', then
         'var_name', then the STASH attribute before falling back to
         the value of `default` (which itself defaults to 'unknown').
 
+        Kwargs:
+
+        * default:
+            The value of the default name.
+        * token:
+            If true, ensure that the name returned satisfies the criteria for
+            the characters required by a valid NetCDF name. If it is not
+            possible to return a valid name, then a ValueError exception is
+            raised.
+
+        Returns:
+            String.
+
         """
-        return self.standard_name or self.long_name or self.var_name or \
-            str(self.attributes.get('STASH', '')) or default
+        def _check(item):
+            return self.token(item) if token else item
+
+        default = self._DEFAULT_NAME if default is None else default
+
+        result = (_check(self.standard_name) or _check(self.long_name) or
+                  _check(self.var_name) or
+                  _check(str(self.attributes.get('STASH', ''))) or
+                  _check(default))
+
+        if token and result is None:
+            emsg = 'Cannot retrieve a valid name token from {!r}'
+            raise ValueError(emsg.format(self))
+
+        return result
 
     def rename(self, name):
         """
@@ -144,12 +196,10 @@ def var_name(self):
     @var_name.setter
     def var_name(self, name):
         if name is not None:
-            if not name:
-                raise ValueError('An empty string is not a valid netCDF '
-                                 'variable name.')
-            elif set(name).intersection(string.whitespace):
-                raise ValueError('{!r} is not a valid netCDF variable name '
-                                 'as it contains whitespace.'.format(name))
+            result = self.token(name)
+            if result is None or not name:
+                emsg = '{!r} is not a valid NetCDF variable name.'
+                raise ValueError(emsg.format(name))
         self._var_name = name
 
     @property

lib/iris/coords.py

@@ -2247,16 +2247,18 @@ def __init__(self, method, coords=None, intervals=None, comments=None):
             raise TypeError("'method' must be a string - got a '%s'" %
                             type(method))
 
+        default_name = CFVariableMixin._DEFAULT_NAME
         _coords = []
         if coords is None:
             pass
         elif isinstance(coords, Coord):
-            _coords.append(coords.name())
+            _coords.append(coords.name(token=True))
         elif isinstance(coords, six.string_types):
-            _coords.append(coords)
+            _coords.append(CFVariableMixin.token(coords) or default_name)
         else:
-            normalise = (lambda coord: coord.name() if
-                         isinstance(coord, Coord) else coord)
+            normalise = (lambda coord: coord.name(token=True) if
+                         isinstance(coord, Coord) else
+                         CFVariableMixin.token(coord) or default_name)
             _coords.extend([normalise(coord) for coord in coords])
 
         _intervals = []

lib/iris/tests/unit/coords/test_CellMethod.py

@@ -0,0 +1,106 @@
+# (C) British Crown Copyright 2019, Met Office
+#
+# This file is part of Iris.
+#
+# Iris is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Lesser General Public License as published by the
+# Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Iris is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with Iris.  If not, see <http://www.gnu.org/licenses/>.
+"""
+Unit tests for the :class:`iris.coords.CellMethod`.
+"""
+
+from __future__ import (absolute_import, division, print_function)
+from six.moves import (filter, input, map, range, zip)  # noqa
+
+# Import iris.tests first so that some things can be initialised before
+# importing anything else.
+import iris.tests as tests
+
+from iris._cube_coord_common import CFVariableMixin
+from iris.coords import CellMethod, Coord
+
+
+class Test(tests.IrisTest):
+    def setUp(self):
+        self.method = 'mean'
+
+    def _check(self, token, coord, default=False):
+        result = CellMethod(self.method, coords=coord)
+        token = token if not default else CFVariableMixin._DEFAULT_NAME
+        expected = '{}: {}'.format(self.method, token)
+        self.assertEqual(str(result), expected)
+
+    def test_coord_standard_name(self):
+        token = 'air_temperature'
+        coord = Coord(1, standard_name=token)
+        self._check(token, coord)
+
+    def test_coord_long_name(self):
+        token = 'long_name'
+        coord = Coord(1, long_name=token)
+        self._check(token, coord)
+
+    def test_coord_long_name_default(self):
+        token = 'long name'  # includes space
+        coord = Coord(1, long_name=token)
+        self._check(token, coord, default=True)
+
+    def test_coord_var_name(self):
+        token = 'var_name'
+        coord = Coord(1, var_name=token)
+        self._check(token, coord)
+
+    def test_coord_var_name_fail(self):
+        token = 'var name'  # includes space
+        emsg = 'is not a valid NetCDF variable name'
+        with self.assertRaisesRegexp(ValueError, emsg):
+            Coord(1, var_name=token)
+
+    def test_coord_stash(self):
+        token = 'stash'
+        coord = Coord(1, attributes=dict(STASH=token))
+        self._check(token, coord)
+
+    def test_coord_stash_default(self):
+        token = '_stash'  # includes leading underscore
+        coord = Coord(1, attributes=dict(STASH=token))
+        self._check(token, coord, default=True)
+
+    def test_string(self):
+        token = 'air_temperature'
+        result = CellMethod(self.method, coords=token)
+        expected = '{}: {}'.format(self.method, token)
+        self.assertEqual(str(result), expected)
+
+    def test_string_default(self):
+        token = 'air temperature'  # includes space
+        result = CellMethod(self.method, coords=token)
+        expected = '{}: unknown'.format(self.method)
+        self.assertEqual(str(result), expected)
+
+    def test_mixture(self):
+        token = 'air_temperature'
+        coord = Coord(1, standard_name=token)
+        result = CellMethod(self.method, coords=[coord, token])
+        expected = '{}: {}, {}'.format(self.method, token, token)
+        self.assertEqual(str(result), expected)
+
+    def test_mixture_default(self):
+        token = 'air temperature'  # includes space
+        coord = Coord(1, long_name=token)
+        result = CellMethod(self.method, coords=[coord, token])
+        expected = '{}: unknown, unknown'.format(self.method, token, token)
+        self.assertEqual(str(result), expected)
+
+
+if __name__ == '__main__':
+    tests.main()

lib/iris/tests/unit/cube_coord_common/__init__.py

@@ -0,0 +1,20 @@
+# (C) British Crown Copyright 2019, Met Office
+#
+# This file is part of Iris.
+#
+# Iris is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Lesser General Public License as published by the
+# Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Iris is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with Iris.  If not, see <http://www.gnu.org/licenses/>.
+"""Unit tests for the :mod:`iris._cube_coord_common` module."""
+
+from __future__ import (absolute_import, division, print_function)
+from six.moves import (filter, input, map, range, zip)  # noqa