Repository: NCAS-CMS/cfdm

Issue:
Zero-sized unlimited dimension when read from a grouped netCDF file  
The file `cm4twc_dump_file.nc` contains subgroups and has an unlimited dimension, currently of size 13. However, when read it gives the unlimited dimension as size 0:

```python
>>> cfdm.read('cm4twc_dump_file.nc')[0]
<Field: transfer_i(time(0), altitude(1), latitude(4), longitude(3)) 1>
```
And downstream errors occur, e.g. when trying to subspace the size zero dimension.

This is a bug in `cfdm.read` (well `cfdm.read_write.netcdf.NetCDFRead` to be exact), which takes the dimension sizes from the _flattened_ version of the file, but the flattened file does not know the unlimited dimension size, because the flattened file contains no arrays. 

This is easily fixed by getting the dimension size from the original grouped file instead. PR to follow.

```python
>>> cfdm.environment(paths=False)
Platform: Linux-5.4.0-62-generic-x86_64-with-debian-bullseye-sid
HDF5 library: 1.10.5
netcdf library: 4.6.3
Python: 3.7.0
netCDF4: 1.5.4
numpy: 1.18.4
cfdm.core: 1.8.8.0
cftime: 1.3.0
netcdf_flattener: 1.2.0
cfdm: 1.8.8.0
```


Discussion:

Patch:
diff --git a/Changelog.rst b/Changelog.rst
index ca53e24bb..09a754780 100644
--- a/Changelog.rst
+++ b/Changelog.rst
@@ -1,3 +1,12 @@
+version NEXT
+------------
+----
+
+**2021-??-??**
+
+* Fix for zero-sized unlimited dimensions when read from a grouped
+  netCDF file (https://github.com/NCAS-CMS/cfdm/issues/113)
+
 version 1.8.8.0
 ---------------
 ----
diff --git a/cfdm/core/__init__.py b/cfdm/core/__init__.py
index edbaa0db6..970af7907 100644
--- a/cfdm/core/__init__.py
+++ b/cfdm/core/__init__.py
@@ -3,9 +3,9 @@
 '''
 
 __author__ = 'David Hassell'
-__date__ = '2020-12-18'
+__date__ = '2020-01-15'
 __cf_version__ = '1.8'
-__version__ = '1.8.8.0'
+__version__ = '1.8.8.1'
 
 from distutils.version import LooseVersion
 import platform
diff --git a/cfdm/read_write/netcdf/netcdfread.py b/cfdm/read_write/netcdf/netcdfread.py
index 4d5147610..4b1287f12 100644
--- a/cfdm/read_write/netcdf/netcdfread.py
+++ b/cfdm/read_write/netcdf/netcdfread.py
@@ -288,6 +288,10 @@ class NetCDFRead(IORead):
         for flat_file in self.read_vars['flat_files']:
             flat_file.close()
 
+        # Close the original grouped file (v1.8.8.1)
+        if 'nc_grouped' in self.read_vars:
+            self.read_vars['nc_grouped'].close()
+
     def file_open(self, filename, flatten=True, verbose=None):
         '''Open the netCDf file for reading.
 
@@ -341,6 +345,7 @@ class NetCDFRead(IORead):
             flat_nc = netCDF4.Dataset(flat_file, 'w',
                                       diskless=True,
                                       persist=False)
+
             flat_nc.set_fill_off()
 
             # Flatten the file
@@ -348,7 +353,12 @@ class NetCDFRead(IORead):
                                      lax_mode=True,
                                      _copy_data=False)
 
-            nc.close()
+            # Store the original grouped file. This is primarily
+            # because the unlimited dimensions in the flattened
+            # dataset have size 0, since it contains no
+            # data. (v1.8.8.1)
+            g['nc_grouped'] = nc
+
             nc = flat_nc
 
             g['has_groups'] = True
@@ -831,6 +841,7 @@ class NetCDFRead(IORead):
         variable_groups = {}
         variable_group_attributes = {}
         variable_basename = {}
+        variable_grouped_dataset = {}
 
         dimension_groups = {}
         dimension_basename = {}
@@ -970,6 +981,8 @@ class NetCDFRead(IORead):
                     # variable path
                     ncvar = ncvar[1:]
                     flattener_variables[ncvar] = ncvar
+
+                variable_grouped_dataset[ncvar] = g['nc_grouped']
             # --- End: if
 
             variable_attributes[ncvar] = {}
@@ -1057,6 +1070,10 @@ class NetCDFRead(IORead):
         # The netCDF4 dataset object for each variable
         g['variable_dataset'] = variable_dataset
 
+        # The original gouped dataset for each variable (empty if the
+        # original dataset is not grouped) v1.8.8.1
+        g['variable_grouped_dataset'] = variable_grouped_dataset
+
         # The name of the file containing the each variable
         g['variable_filename'] = variable_filename
 
@@ -1074,7 +1091,19 @@ class NetCDFRead(IORead):
         # The netCDF dimensions of the parent file
         internal_dimension_sizes = {}
         for name, dimension in nc.dimensions.items():
-            internal_dimension_sizes[name] = dimension.size
+            if (
+                    has_groups
+                    and dimension_isunlimited[flattener_dimensions[name]]
+            ):
+                # For grouped datasets, get the unlimited dimension
+                # size from the original grouped dataset, because
+                # unlimited dimensions have size 0 in the flattened
+                # dataset (because it contains no data) (v1.8.8.1)
+                group, ncdim = self._netCDF4_group(g['nc_grouped'],
+                                                   flattener_dimensions[name])
+                internal_dimension_sizes[name] = group.dimensions[ncdim].size
+            else:
+                internal_dimension_sizes[name] = dimension.size
 
         if g['has_groups']:
             internal_dimension_sizes = {
@@ -4579,7 +4608,20 @@ class NetCDFRead(IORead):
         '''
         g = self.read_vars
 
-        variable = g['variables'].get(ncvar)
+        if g['has_groups']:
+            # Get the variable from the original grouped file. This is
+            # primarily so that unlimited dimensions don't come out
+            # with size 0 (v1.8.8.1)
+            group, name = self._netCDF4_group(
+                g['variable_grouped_dataset'][ncvar], ncvar
+            )
+#            path = ncvar.split('/')
+#            for group_name in path[1:-1]:
+#                group = group[group_name]
+            variable = group.variables.get(name)
+        else:
+            variable = g['variables'].get(ncvar)
+
         if variable is None:
             return None
 
@@ -4596,6 +4638,7 @@ class NetCDFRead(IORead):
         ndim = variable.ndim
         shape = variable.shape
         size = variable.size
+
         if size < 2:
             size = int(size)
 
@@ -6262,4 +6305,37 @@ class NetCDFRead(IORead):
 
         return out
 
+    def _netCDF4_group(self, nc, name):
+        '''Given a dataset and a variable or dimension name, return the group
+object for the name, and the name within the group.
+
+    .. versionadded:: 1.8.8.1
+
+    :Parameters:
+
+        nc: `netCDF4._netCDF4.Dataset` or `netCDF4._netCDF4.Group`
+
+        name: `str`
+
+    :Returns:
+
+        `netCDF4._netCDF4.Dataset` or `netCDF4._netCDF4.Group`, `str`
+
+    **Examples:**
+
+    >>> group, name = n._netCDF4_group(nc, 'time')
+    >>> group.name, name
+    ('/', 'time')
+    >>> group, name = n._netCDF4_group(nc, '/surfacelayer/Z')
+    >>> group.name, name
+    ('surfacelayer', 'Z')
+
+        '''
+        group = nc
+        path = name.split('/')
+        for group_name in path[1:-1]:
+            group = group[group_name]
+
+        return group, path[-1]
+
 # --- End: class
diff --git a/docs/source/contributing.rst b/docs/source/contributing.rst
index 6c44f87dc..c9dc1dc7b 100644
--- a/docs/source/contributing.rst
+++ b/docs/source/contributing.rst
@@ -102,6 +102,7 @@ a work in progress, you can mark it initially as a draft pull request.
 We would like to acknowledge and thank all those who have contributed
 ideas, code, and documentation to the cfdm library:
 
+* Alan Iwi
 * Allyn Treshansky
 * Bryan Lawrence
 * David Hassell


Test patch:
diff --git a/cfdm/test/test_groups.py b/cfdm/test/test_groups.py
index da77ea373..bf7c8052c 100644
--- a/cfdm/test/test_groups.py
+++ b/cfdm/test/test_groups.py
@@ -9,7 +9,7 @@ import netCDF4
 import cfdm
 
 
-n_tmpfiles = 8
+n_tmpfiles = 9
 tmpfiles = [tempfile.mkstemp('_test_groups.nc', dir=os.getcwd())[1]
             for i in range(n_tmpfiles)]
 (
@@ -21,6 +21,7 @@ tmpfiles = [tempfile.mkstemp('_test_groups.nc', dir=os.getcwd())[1]
     grouped_file2,
     grouped_file3,
     grouped_file4,
+    grouped_file5,
 ) = tmpfiles
 
 
@@ -405,6 +406,42 @@ class GroupsTest(unittest.TestCase):
         h = h[0]
         self.assertTrue(f.equals(h, verbose=3))
 
+    def test_groups_unlimited_dimension(self):
+        f = cfdm.example_field(0)
+
+        # Create an unlimited dimension in the root group
+        key = f.domain_axis_key('time')
+        domain_axis = f.constructs[key]
+        domain_axis.nc_set_unlimited(True)
+
+        f.insert_dimension(key, 0, inplace=True)
+
+        key = f.domain_axis_key('latitude')
+        domain_axis = f.constructs[key]
+        domain_axis.nc_set_unlimited(True)
+        domain_axis.nc_set_dimension_groups(['forecast'])
+
+        # ------------------------------------------------------------
+        # Move the latitude coordinate to the /forecast group. Note
+        # that this will drag its netDF dimension along with it,
+        # because it's a dimension coordinate variable
+        # ------------------------------------------------------------
+        lat = f.construct('latitude')
+        lat.nc_set_variable_groups(['forecast'])
+
+        # ------------------------------------------------------------
+        # Move the field construct to the /forecast/model group
+        # ------------------------------------------------------------
+        f.nc_set_variable_groups(['forecast', 'model'])
+
+        grouped_file = grouped_file5
+        cfdm.write(f, grouped_file5, verbose=1)
+
+        h = cfdm.read(grouped_file, verbose=1)
+        self.assertEqual(len(h), 1)
+        h = h[0]
+        self.assertTrue(f.equals(h, verbose=3))
+
 # --- End: class