Repository: OceanParcels/parcels
Pull request #545 (2019-02-25 08:23:59)

Test correct keys in dimensions dictionary
This PR tests that the `dimensions` dictionary has no other keys than `lon`, `lat`, `depth` and `time`. Since other keys are not used, this prevents errors where users use the wrong key for e.g. `depth` and inadvertently use in 2D mode

Furthermore, this PR also cleans up the code by removing the `dimensions['data']` entry for the name of the variable in the filebuffer


Patch:
diff --git a/parcels/field.py b/parcels/field.py
index ee1dac1b..a18e0a60 100644
--- a/parcels/field.py
+++ b/parcels/field.py
@@ -55,7 +55,11 @@ class Field(object):
     def __init__(self, name, data, lon=None, lat=None, depth=None, time=None, grid=None, mesh='flat',
                  fieldtype=None, transpose=False, vmin=None, vmax=None, time_origin=None,
                  interp_method='linear', allow_time_extrapolation=None, time_periodic=False, **kwargs):
-        self.name = name
+        if not isinstance(name, tuple):
+            self.name = name
+            self.filebuffername = name
+        else:
+            self.name, self.filebuffername = name
         self.data = data
         time_origin = TimeConverter(0) if time_origin is None else time_origin
         if grid:
@@ -77,8 +81,8 @@ class Field(object):
         else:
             raise ValueError("Unsupported mesh type. Choose either: 'spherical' or 'flat'")
         if type(interp_method) is dict:
-            if name in interp_method:
-                self.interp_method = interp_method[name]
+            if self.name in interp_method:
+                self.interp_method = interp_method[self.name]
             else:
                 raise RuntimeError('interp_method is a dictionary but %s is not in it' % name)
         else:
@@ -133,7 +137,7 @@ class Field(object):
                filenames can be a list [files]
                or a dictionary {dim:[files]} (if lon, lat, depth and/or data not stored in same files as data)
                time values are in filenames[data]
-        :param variable: Name of the field to create. Note that this has to be a string
+        :param variable: Tuple mapping field name to variable name in the NetCDF file.
         :param dimensions: Dictionary mapping variable names for the relevant dimensions in the NetCDF file
         :param indices: dictionary mapping indices for each dimension to read from file.
                This can be used for reading in only a subregion of the NetCDF file.
@@ -162,6 +166,9 @@ class Field(object):
             data_filenames = variable
             netcdf_engine = 'xarray'
         else:
+            if isinstance(variable, str):  # for backward compatibility with Parcels < 2.0.0
+                variable = (variable, variable)
+            assert len(variable) == 2, 'The variable tuple must have length 2. Use FieldSet.from_netcdf() for multiple variables'
             if not isinstance(filenames, Iterable) or isinstance(filenames, str):
                 filenames = [filenames]
 
@@ -254,7 +261,7 @@ class Field(object):
                     if netcdf_engine == 'xarray':
                         tslice = [tslice]
                     else:
-                        filebuffer.name = filebuffer.parse_name(dimensions, variable)
+                        filebuffer.name = filebuffer.parse_name(variable[1])
 
                     if len(filebuffer.data.shape) == 2:
                         data[ti:ti+len(tslice), 0, :, :] = filebuffer.data
@@ -896,7 +903,7 @@ class Field(object):
             time_data = g.time_origin.reltime(time_data)
             filebuffer.ti = (time_data <= g.time[tindex]).argmin() - 1
             if self.netcdf_engine != 'xarray':
-                filebuffer.name = filebuffer.parse_name(self.dimensions, self.name)
+                filebuffer.name = filebuffer.parse_name(self.filebuffername)
             if len(filebuffer.data.shape) == 2:
                 data[tindex, 0, :, :] = filebuffer.data
             elif len(filebuffer.data.shape) == 3:
@@ -1338,8 +1345,7 @@ class NetcdfFileBuffer(object):
         else:
             self.dataset.close()
 
-    def parse_name(self, dimensions, variable):
-        name = dimensions['data'] if 'data' in dimensions else variable
+    def parse_name(self, name):
         if isinstance(name, list):
             for nm in name:
                 if hasattr(self.dataset, nm):
diff --git a/parcels/fieldset.py b/parcels/fieldset.py
index 96a3c111..d5347e8a 100644
--- a/parcels/fieldset.py
+++ b/parcels/fieldset.py
@@ -34,6 +34,12 @@ class FieldSet(object):
 
         self.compute_on_defer = None
 
+    @staticmethod
+    def checkvaliddimensionsdict(dims):
+        for d in dims:
+            if d not in ['lon', 'lat', 'depth', 'time']:
+                raise NameError('%s is not a valid key in the dimensions dictionary' % d)
+
     @classmethod
     def from_data(cls, data, dimensions, transpose=False, mesh='spherical',
                   allow_time_extrapolation=None, time_periodic=False, **kwargs):
@@ -71,6 +77,7 @@ class FieldSet(object):
         for name, datafld in data.items():
             # Use dimensions[name] if dimensions is a dict of dicts
             dims = dimensions[name] if name in dimensions else dimensions
+            cls.checkvaliddimensionsdict(dims)
 
             if allow_time_extrapolation is None:
                 allow_time_extrapolation = False if 'time' in dims else True
@@ -225,7 +232,7 @@ class FieldSet(object):
 
             # Use dimensions[var] and indices[var] if either of them is a dict of dicts
             dims = dimensions[var] if var in dimensions else dimensions
-            dims['data'] = name
+            cls.checkvaliddimensionsdict(dims)
             inds = indices[var] if (indices and var in indices) else indices
 
             grid = None
@@ -239,14 +246,14 @@ class FieldSet(object):
                         sameGrid = True
                     elif type(filenames[procvar]) == dict:
                         sameGrid = True
-                        for dim in ['lon', 'lat', 'depth', 'data']:
+                        for dim in ['lon', 'lat', 'depth']:
                             if dim in dimensions:
                                 sameGrid *= filenames[procvar][dim] == filenames[var][dim]
                     if sameGrid:
                         grid = fields[procvar].grid
                         kwargs['dataFiles'] = fields[procvar].dataFiles
                         break
-            fields[var] = Field.from_netcdf(paths, var, dims, inds, grid=grid, mesh=mesh,
+            fields[var] = Field.from_netcdf(paths, (var, name), dims, inds, grid=grid, mesh=mesh,
                                             allow_time_extrapolation=allow_time_extrapolation,
                                             time_periodic=time_periodic, full_load=full_load, **kwargs)
         u = fields.pop('U', None)


Test patch:
diff --git a/tests/test_fieldset.py b/tests/test_fieldset.py
index ae414f32..d84f5f52 100644
--- a/tests/test_fieldset.py
+++ b/tests/test_fieldset.py
@@ -126,6 +126,26 @@ def test_fieldset_from_file_subsets(indslon, indslat, tmpdir, filename='test_sub
     assert np.allclose(fieldsetsub.V.data, fieldsetfull.V.data[ixgrid])
 
 
+@pytest.mark.parametrize('calltype', ['from_data', 'from_nemo'])
+def test_illegal_dimensionsdict(calltype):
+    error_thrown = False
+    try:
+        if calltype == 'from_data':
+            data, dimensions = generate_fieldset(10, 10)
+            dimensions['test'] = None
+            FieldSet.from_data(data, dimensions)
+        elif calltype == 'from_nemo':
+            fname = path.join(path.dirname(__file__), 'test_data', 'mask_nemo_cross_180lon.nc')
+            filenames = {'dx': fname, 'mesh_mask': fname}
+            variables = {'dx': 'e1u'}
+            dimensions = {'lon': 'glamu', 'lat': 'gphiu', 'test': 'test'}
+            error_thrown = False
+            FieldSet.from_nemo(filenames, variables, dimensions)
+    except NameError:
+        error_thrown = True
+    assert error_thrown
+
+
 @pytest.mark.parametrize('xdim', [100, 200])
 @pytest.mark.parametrize('ydim', [100, 200])
 def test_add_field(xdim, ydim, tmpdir, filename='test_add'):