Repository: pydap/pydap

Issue:
Netcdf handler errs when defining (named) dimensions within nested groups
<!-- Thank you very much for your input! -->

### Description

Consider the scenario of an empty dataset with the following structure:

```python
ds.tree()
```
```
.root
|--group1
   |--subgroup1
```
but with named dimensions defined by their name and a size. That is:

```python
ds['/group1'].dimensions = {'lat': 2}
ds['/group1/subgroup1'].dimensions={'lat':1}
```

In such case `lat` is a named dimensions within `Group1` and `lat` is another named dimension within `group2/subgroup1`. However, they DO NOT have the same (fully qualifying) name: `/group1/lat` and `/group1/subgroup1/lat`, and so technically this is permitted within the netcdf4/DAP4 data model.

## issue 1)

Currently there is an issue with the netcdf handler with named dimensions in the following snippet

https://github.com/pydap/pydap/blob/09a3348f56914750b86ec09d472ba685d69e4918/src/pydap/handlers/netcdf/__init__.py#L144


This assumes there is a variable with the same name as the dimension exists. And so the scenario above errs with named dimensions (no variable with same name). Instead, the correct approach should be:

```python
        Dims.update({_source[group].dimensions[dim].name: _source[group].dimensions[dim].size})
```
This is the correct approach because we are only interested in the dimension name and size (attributes of the Group object) and not on the variable itself. As it stands, the approach above only works with coordinate dimensions (variables that share the same name as the dimension) and errs when there are named dimensions only.


## issue 2)

The example above also highlights another issue: two named dimensions with identical (relative to its path) name. Even though

```python
ds['/group1'].dimensions = {'lat': 2}
ds['/group1/subgroup1'].dimensions={'lat':1}
```

could cause a conflict at first sight, this is permitted within the netCDF4 and DAP4 model because the fully qualifying name are `/group1/lat` and `/group1/subgroup1/lat`. However the following snippet:

https://github.com/pydap/pydap/blob/09a3348f56914750b86ec09d472ba685d69e4918/src/pydap/handlers/netcdf/__init__.py#L142-L143

in the case their relative names are the same, makes it so that `/group1/subgroup1/lat` is not added to the dictionary because the key `lat` already exists. And so in the case there is a variable inside subgroup1 with relative dimension `lat`, when doing:

```python
ds['group1/subgroup1/subgroup1_var'].dimensions
```
is empty. Instead it should yield: `ds['group1/subgroup1/subgroup1_var'].dimensions = ('/group1/subgroup1/lat')`.


### minimal example

The following netcdf file reproduces the two errors:

```python

    with nc4.Dataset(filepath, "w", format="NETCDF4") as root_group:
        group_1 = root_group.createGroup("/Group1")
        subgroup_1 = group_1.createGroup("/subgroup1")

        root_group.createDimension("lat", 1)
        root_group.createDimension("lon", 2)
        root_group.createVariable("root_variable", np.float64, ("lat", "lon"))

        group_1_var = group_1.createVariable("group_1_var", np.float64, ("lat", "lon"))
        group_1_var[:] = np.array([[0.1, 0.2]])
        group_1_var.units = "K"
        group_1_var.long_name = "air_temperature"

        subgroup_1.createDimension("lat", 2)

        subgroup1_var = subgroup_1.createVariable(
            "subgroup1_var", np.float64, ("lat", "lon")
        )
        subgroup1_var[:] = np.array([[0.1, 0.2]])
```





Discussion:

Patch:
diff --git a/src/pydap/handlers/netcdf/__init__.py b/src/pydap/handlers/netcdf/__init__.py
index a20cc9e..ae9e53b 100644
--- a/src/pydap/handlers/netcdf/__init__.py
+++ b/src/pydap/handlers/netcdf/__init__.py
@@ -65,7 +65,7 @@ class NetCDFHandler(BaseHandler):
                             "Unlimited_Dimension": dim,
                         }
                     else:
-                        Dims.update({source.path + dims[dim].name: dims[dim].size})
+                        Dims.update({dims[dim].name: dims[dim].size})
                 # build dataset
                 name = os.path.split(filepath)[1]
                 self.dataset = DatasetType(
@@ -98,7 +98,7 @@ class NetCDFHandler(BaseHandler):
                     # start at root level
                     path = source.path
                     for vdim in source.dimensions:
-                        fqn_dims.update({vdim: path + vdim})
+                        fqn_dims.update({path + vdim: vdim})  # fqn is unique
                     fqn_dims = group_fqn(self.dataset, source, fqn_dims)
 
                 vdims = [dim for dim in dims if dim in vars]
@@ -137,11 +137,19 @@ def group_fqn(_dataset, _source, _fqn_dims=OrderedDict()):
             _path = _path + "/"
         # create group and attrs + dims (non-fqn)
         dims = _source[group].dimensions
-        Dims = {}
+        Dims = OrderedDict()
         for dim in dims:
-            if dim not in _fqn_dims.keys():
-                _fqn_dims.update({dim: _path + dim})
-            Dims.update({_source[group][dim].name: _source[group][dim].size})
+            if dim not in _fqn_dims.items():
+                _fqn_dims.update({_path + dim: dim})
+            Dims.update(
+                {
+                    _source[group]
+                    .dimensions[dim]
+                    .name: _source[group]
+                    .dimensions[dim]
+                    .size
+                }
+            )
         _attrs = dict(
             (attr, _source[group].getncattr(attr)) for attr in _source[group].ncattrs()
         )
@@ -153,12 +161,17 @@ def group_fqn(_dataset, _source, _fqn_dims=OrderedDict()):
             dims = list(_source[group][var].dimensions)  # these must have fqn
             vdims = []  # create mapping for fqn
             for dim in dims:
-                vdims.append(_fqn_dims[dim])
+                for key, value in _fqn_dims.items():
+                    if dim == value:
+                        match_dim = key  # overwrites until the last one (most recent)
+                vdims.append(match_dim)
             vattrs = dict(
                 (attr, _source[group][var].getncattr(attr))
                 for attr in _source[group][var].ncattrs()
             )
-            _dataset.createVariable(_path + var, data=data, dims=tuple(vdims), **vattrs)
+            _dataset.createVariable(
+                _path + var, data=data, path=_path, dimensions=tuple(vdims), **vattrs
+            )
         # check if there are nested group
         if len(_source[group].groups) > 0:
             _fqn_dims = group_fqn(_dataset, _source[group], _fqn_dims)
diff --git a/src/pydap/parsers/dmr.py b/src/pydap/parsers/dmr.py
index e67f358..079be45 100644
--- a/src/pydap/parsers/dmr.py
+++ b/src/pydap/parsers/dmr.py
@@ -191,6 +191,7 @@ def dmr_to_dataset(dmr):
     global_dimensions = []
     for name, size in named_dimensions.items():
         if len(name.split("/")) == 1:
+            print(name)
             global_dimensions.append([name, size])
 
     dataset.dimensions = {k: v for k, v in global_dimensions}


Test patch:
diff --git a/src/pydap/tests/test_handlers_netcdf.py b/src/pydap/tests/test_handlers_netcdf.py
index a081ce9..8f89dca 100644
--- a/src/pydap/tests/test_handlers_netcdf.py
+++ b/src/pydap/tests/test_handlers_netcdf.py
@@ -76,6 +76,31 @@ def nested_group_array_file(simple_Group_data, tmpdir_factory):
     return file_name
 
 
+@pytest.fixture(scope="module")
+def nested_group_repeatdims_file(tmpdir_factory):
+    file_name = str(tmpdir_factory.mktemp("nc").join("unaligned_subgroups.nc"))
+    with Dataset(file_name, "w", format="NETCDF4") as root_group:
+        group_1 = root_group.createGroup("/Group1")
+        subgroup_1 = group_1.createGroup("/subgroup1")
+
+        root_group.createDimension("lat", 1)
+        root_group.createDimension("lon", 2)
+        root_group.createVariable("root_variable", np.float64, ("lat", "lon"))
+
+        group_1_var = group_1.createVariable("group_1_var", np.float64, ("lat", "lon"))
+        group_1_var[:] = np.array([[0.1, 0.2]])
+        group_1_var.units = "K"
+        group_1_var.long_name = "air_temperature"
+
+        subgroup_1.createDimension("lat", 2)
+
+        subgroup1_var = subgroup_1.createVariable(
+            "subgroup1_var", np.float64, ("lat", "lon")
+        )
+        subgroup1_var[:] = np.array([[0.1, 0.2]])
+    return file_name
+
+
 @pytest.fixture(scope="module")
 def simple_handler(simple_nc_file):
     return NetCDFHandler(simple_nc_file)
@@ -91,6 +116,11 @@ def simple_handler3(nested_group_array_file):
     return NetCDFHandler(nested_group_array_file)
 
 
+@pytest.fixture(scope="module")
+def simple_handler4(nested_group_repeatdims_file):
+    return NetCDFHandler(nested_group_repeatdims_file)
+
+
 def test_handler(simple_data, simple_handler):
     """Test that dataset has the correct data proxies for grids."""
     dataset = simple_handler.dataset
@@ -114,9 +144,9 @@ def test_handler_array(simple_Group_data, simple_handler2):
     g_dims = dataset["Group"].attributes["dimensions"]
     temp = dataset["/Group/temperature"][:]
     np.testing.assert_array_equal(temp.data, simple_Group_data)
-    assert r_dims == {"/time": 1, "/nv": 2}
+    assert r_dims == {"time": 1, "nv": 2}
     assert g_dims == {"X": 4, "Y": 4}
-    assert temp.dims == ("/time", "/Group/Y", "/Group/X")
+    assert temp.dimensions == ("/time", "/Group/Y", "/Group/X")
 
 
 def test_handler_nested_Group_array(simple_Group_data, simple_handler3):
@@ -127,10 +157,25 @@ def test_handler_nested_Group_array(simple_Group_data, simple_handler3):
     sg_dims = dataset["/Group/SubGroup"].attributes["dimensions"]
     temp = dataset["/Group/SubGroup/temperature"][:]
     np.testing.assert_array_equal(temp.data, simple_Group_data)
-    assert r_dims == {"/time": 1}
+    assert r_dims == {"time": 1}
     assert g_dims == {}
     assert sg_dims == {"X": 4, "Y": 4}
-    assert temp.dims == ("/time", "/Group/SubGroup/Y", "/Group/SubGroup/X")
+    assert temp.dimensions == ("/time", "/Group/SubGroup/Y", "/Group/SubGroup/X")
+
+
+def test_handler_repeatdims_Group_array(simple_handler4):
+    """Test that dataset has the correct data proxies for grids."""
+    dataset = simple_handler4.dataset
+    r_dims = dataset.dimensions
+    g_dims = dataset["Group1"].dimensions
+    var_g1 = dataset["Group1/group_1_var"].dimensions
+    sg_dims = dataset["Group1/subgroup1"].dimensions
+    var_sg1 = dataset["Group1/subgroup1/subgroup1_var"].dimensions
+    assert r_dims == {"lat": 1, "lon": 2}
+    assert g_dims == {}
+    assert sg_dims == {"lat": 2}
+    assert var_g1 == ("/lat", "/lon")
+    assert var_sg1 == ("/Group1/subgroup1/lat", "/lon")
 
 
 @pytest.fixture(scope="module")