Repository: pydap/pydap

Issue:
refactor to resolve hybrid DAP2/DAP4 behavior on netcdf handler when using as server
<!-- Thank you very much for your input! -->

### Description
Pydap 's netcdf handler was updated to now serve nc4 data that contains groups (per this PR #367). But pydap's netcdf handler follows a hybrid DAP2/DAP4 model, to both allow data within Groups, while remain compatible with GridType. This results in

a) an inefficient code that, in order to generate dds, das and dmr responses, runs unnecessary extra parser code. That is, there is a parser for `dds`, another one for `das` and another one for `dmr` and these are largely independent from each other.

b) When serving nc4 dataset, the dmr can result in `attribute errors` when looking dimension variables that are defined at the `root` level (for example, when serving the [SimpleGroup](https://github.com/pydap/pydap/blob/8d36407c2b7b30cf29739faedefecbfa2f31762b/src/pydap/tests/datasets.py#L220-L257) dataset), resulting in incomplete DMR responses. This is because anything at the root level in the netcdf handler was left unmodified in PR #367 to remain compatible with the DAP2 data model.

## Solution
The big picture solution is that OPeNDAP wants to move towards the DAP4 as the default model implementation, and so the netcdf handler should be most consistent with DMRs. Moreover, to support DAP2 workflows, the `dds` and `dar` parsers should use the dmr parser internally to create the appropriate DAP metadata response. This implies a refactor of the `dds` and `das` parsers.




Discussion:

Patch:
diff --git a/src/pydap/handlers/netcdf/__init__.py b/src/pydap/handlers/netcdf/__init__.py
index ae9e53b..a553b46 100644
--- a/src/pydap/handlers/netcdf/__init__.py
+++ b/src/pydap/handlers/netcdf/__init__.py
@@ -11,7 +11,7 @@ import numpy as np
 
 from pydap.exceptions import OpenFileError
 from pydap.handlers.lib import BaseHandler
-from pydap.model import BaseType, DatasetType, GridType
+from pydap.model import BaseType, DatasetType
 from pydap.pycompat import suppress
 
 # Check for netCDF4 presence:
@@ -59,7 +59,9 @@ class NetCDFHandler(BaseHandler):
 
                 # Add dimensions when creating the DatasetType
                 Dims = OrderedDict()
+                fqn_dims = OrderedDict()  # keep track of fully qualifying names of dims
                 for dim in dims:
+                    fqn_dims.update({"/" + dim: dim})
                     if dims[dim] is None:
                         self.dataset.attributes["DODS_EXTRA"] = {
                             "Unlimited_Dimension": dim,
@@ -67,6 +69,7 @@ class NetCDFHandler(BaseHandler):
                     else:
                         Dims.update({dims[dim].name: dims[dim].size})
                 # build dataset
+
                 name = os.path.split(filepath)[1]
                 self.dataset = DatasetType(
                     name, dimensions=Dims, attributes=dict(NC_GLOBAL=attrs(source))
@@ -75,31 +78,21 @@ class NetCDFHandler(BaseHandler):
                 # add grids
                 grids = [var for var in vars if var not in dims]
                 for grid in grids:
-                    self.dataset[grid] = GridType(grid, attrs(vars[grid]))
-                    # add array
-                    self.dataset[grid][grid] = BaseType(
+                    # make dimension a fully qualifying name
+                    dimensions = tuple(["/" + dim for dim in vars[grid].dimensions])
+                    self.dataset[grid] = BaseType(
                         grid,
-                        LazyVariable(source, grid, grid, self.filepath),
-                        vars[grid].dimensions,
-                        attrs(vars[grid]),
+                        LazyVariable(source[grid], grid, grid, self.filepath),
+                        dimensions=dimensions,
+                        **attrs(vars[grid]),
                     )
-                    # add maps
-                    for dim in vars[grid].dimensions:
-                        try:
-                            data = vars[dim][:]
-                            attributes = attrs(vars[dim])
-                        except KeyError:
-                            data = np.arange(dims[dim].size, dtype="i")
-                            attributes = None
-                        self.dataset[grid][dim] = BaseType(dim, data, None, attributes)
 
-                fqn_dims = OrderedDict()  # keep track of fully qualifying names of dims
                 if len(source.groups) > 0:
                     # start at root level
                     path = source.path
                     for vdim in source.dimensions:
                         fqn_dims.update({path + vdim: vdim})  # fqn is unique
-                    fqn_dims = group_fqn(self.dataset, source, fqn_dims)
+                    fqn_dims = group_fqn(self.dataset, source, self.filepath, fqn_dims)
 
                 vdims = [dim for dim in dims if dim in vars]
                 for dim in vdims:
@@ -112,7 +105,7 @@ class NetCDFHandler(BaseHandler):
             raise OpenFileError(message)
 
 
-def group_fqn(_dataset, _source, _fqn_dims=OrderedDict()):
+def group_fqn(_dataset, _source, _filepath, _fqn_dims=OrderedDict()):
     """Function to create nested DAP objects with fully-qualified-names
     within a hierarchy. Returns a dictionary with mapping between dimension
     names used at the group/array level and their fully qualifying names.
@@ -153,11 +146,12 @@ def group_fqn(_dataset, _source, _fqn_dims=OrderedDict()):
         _attrs = dict(
             (attr, _source[group].getncattr(attr)) for attr in _source[group].ncattrs()
         )
+
         _dataset.createGroup(_source[group].path, dimensions=Dims, **_attrs)
         # now vars
         Vars = _source[group].variables
         for var in Vars:
-            data = _source[group][var][:].data  # extract data from file
+            data = _source[group][var]
             dims = list(_source[group][var].dimensions)  # these must have fqn
             vdims = []  # create mapping for fqn
             for dim in dims:
@@ -170,20 +164,23 @@ def group_fqn(_dataset, _source, _fqn_dims=OrderedDict()):
                 for attr in _source[group][var].ncattrs()
             )
             _dataset.createVariable(
-                _path + var, data=data, path=_path, dimensions=tuple(vdims), **vattrs
+                _path + var,
+                data=LazyVariable(data, var, _path + var, _filepath),
+                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)
+            _fqn_dims = group_fqn(_dataset, _source[group], _filepath, _fqn_dims)
     return _fqn_dims
 
 
 class LazyVariable:
-    def __init__(self, source, name, path, filepath):
+    def __init__(self, var, name, path, filepath):
         self.filepath = filepath
         self.path = path
-        var = source[self.path]
-        self.dimensions = var._getdims()
+        # var = source[self.path]
+        # self.dimensions = fqn_dims
         self.dtype = np.dtype(var.dtype)
         self.datatype = var.dtype
         self.ndim = len(var.dimensions)
diff --git a/src/pydap/parsers/dmr.py b/src/pydap/parsers/dmr.py
index 079be45..e67f358 100644
--- a/src/pydap/parsers/dmr.py
+++ b/src/pydap/parsers/dmr.py
@@ -191,7 +191,6 @@ 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}
diff --git a/src/pydap/responses/dmr.py b/src/pydap/responses/dmr.py
index dd7f6c0..931093e 100644
--- a/src/pydap/responses/dmr.py
+++ b/src/pydap/responses/dmr.py
@@ -13,14 +13,7 @@ except ImportError:
     from singledispatch import singledispatch
 
 from ..lib import __version__
-from ..model import (
-    BaseType,
-    DatasetType,
-    GridType,
-    GroupType,
-    SequenceType,
-    StructureType,
-)
+from ..model import BaseType, DatasetType, GroupType, SequenceType, StructureType
 from .lib import BaseResponse
 
 INDENT = " " * 4
@@ -115,32 +108,19 @@ def _grouptype(var, level=0):
     yield "{indent}</Group>\n".format(indent=level * INDENT)
 
 
-@dmr.register(GridType)
-def _gridtype(var, level=0):
-    yield '{indent}<{type} name="{name}">\n'.format(
-        indent=level * INDENT,
-        type=var.dtype,
-        name=var.name,
-    )
-
-    # get dimensions
-    for dim in var.maps:
-        yield '{indent}<Dim name="{name}"/>\n'.format(
-            indent=(level + 1) * INDENT, name=dim
-        )
-
-
 @dmr.register(BaseType)
 def _basetype(var, level=0):
     _ntype = var.dtype
     _vartype = str(_ntype)[0].upper() + str(_ntype)[1:]
+    if _vartype == "<U0":
+        _vartype = "String"
     yield '{indent}<{type} name="{name}">\n'.format(
         indent=level * INDENT,
         type=_vartype,
         name=var.name,
     )
     # get dimensions
-    for dim in var.dims:
+    for dim in var.dimensions:
         yield '{indent}<Dim name="{name}"/>\n'.format(
             indent=(level + 1) * INDENT, name=dim
         )


Test patch:
diff --git a/src/pydap/tests/datasets.py b/src/pydap/tests/datasets.py
index 9661331..734a1d8 100644
--- a/src/pydap/tests/datasets.py
+++ b/src/pydap/tests/datasets.py
@@ -231,33 +231,33 @@ SimpleGroup.createVariable(
     name="/SimpleGroup/Temperature",
     data=np.arange(10, 26, 1, dtype="f4").reshape(1, 4, 4),
     units="degrees_celsius",
-    dims=("/time", "/SimpleGroup/Y", "/SimpleGroup/X"),
+    dimensions=("/time", "/SimpleGroup/Y", "/SimpleGroup/X"),
     _FillValue=np.inf,
 )
 SimpleGroup.createVariable(
     name="/SimpleGroup/Salinity",
     data=30 * np.ones(16, dtype="f4").reshape(1, 4, 4),
     units="psu",
-    dims=("/time", "/SimpleGroup/Y", "/SimpleGroup/X"),
+    dimensions=("/time", "/SimpleGroup/Y", "/SimpleGroup/X"),
     _FillValue=np.nan,
 )
 SimpleGroup.createVariable(
-    name="/SimpleGroup/Y", data=np.arange(4, dtype="i2"), dims=("/SimpleGroup/Y",)
+    name="/SimpleGroup/Y", data=np.arange(4, dtype="i2"), dimensions=("/SimpleGroup/Y",)
 )
 SimpleGroup.createVariable(
-    name="/SimpleGroup/X", data=np.arange(4, dtype="i2"), dims=("/SimpleGroup/X",)
+    name="/SimpleGroup/X", data=np.arange(4, dtype="i2"), dimensions=("/SimpleGroup/X",)
 )
 SimpleGroup.createVariable(
     name="/time",
     data=np.array(0.5, dtype="f4"),
-    dims=("/time",),
+    dimensions=("/time",),
     attributes={
         "standard_name": "time",
         "bounds": "time_bnds",
     },
 )
 SimpleGroup.createVariable(
-    name="/time_bnds", data=np.arange(2, dtype="f4"), dims=("/time", "/nv")
+    name="/time_bnds", data=np.arange(2, dtype="f4"), dimensions=("/time", "/nv")
 )
 
 
diff --git a/src/pydap/tests/test_handlers_netcdf.py b/src/pydap/tests/test_handlers_netcdf.py
index 8f89dca..3a3f64d 100644
--- a/src/pydap/tests/test_handlers_netcdf.py
+++ b/src/pydap/tests/test_handlers_netcdf.py
@@ -4,7 +4,6 @@ import numpy as np
 import pytest
 from netCDF4 import Dataset
 
-from pydap.handlers.dap import DAPHandler
 from pydap.handlers.netcdf import NetCDFHandler
 
 
@@ -128,8 +127,8 @@ def test_handler(simple_data, simple_handler):
     retrieved_data = list(
         zip(
             dataset["index"][:],
-            dataset["temperature"].array[:],
-            dataset["station"].array[:],
+            dataset["temperature"][:],
+            dataset["station"][:],
         )
     )
     np.testing.assert_array_equal(
@@ -185,17 +184,17 @@ def simple_application(simple_handler):
     return ServerSideFunctions(simple_handler)
 
 
-def test_open(simple_data, simple_application):
-    """Test that NetCDFHandler can be read through open_url."""
-    dataset = DAPHandler("http://localhost:8001/", simple_application).dataset
-    dtype = [("index", "<i4"), ("temperature", "<f8"), ("station", "S40")]
-    retrieved_data = list(
-        zip(
-            dataset["index"][:],
-            dataset["temperature"].array[:],
-            dataset["station"].array[:],
-        )
-    )
-    np.testing.assert_array_equal(
-        np.array(retrieved_data, dtype=dtype), np.array(simple_data, dtype=dtype)
-    )
+# def test_open(simple_data, simple_application):
+#     """Test that NetCDFHandler can be read through open_url."""
+#     dataset = DAPHandler("http://localhost:8001/", simple_application).dataset
+#     dtype = [("index", "<i4"), ("temperature", "<f8"), ("station", "S40")]
+#     retrieved_data = list(
+#         zip(
+#             dataset["index"][:],
+#             dataset["temperature"][:],
+#             dataset["station"][:],
+#         )
+#     )
+#     np.testing.assert_array_equal(
+#         np.array(retrieved_data, dtype=dtype), np.array(simple_data, dtype=dtype)
+#     )
diff --git a/src/pydap/tests/test_responses_html.py b/src/pydap/tests/test_responses_html.py
index b3f8fbf..6b147c7 100644
--- a/src/pydap/tests/test_responses_html.py
+++ b/src/pydap/tests/test_responses_html.py
@@ -180,7 +180,7 @@ class TestHTMLResponseSimpleGroup(unittest.TestCase):
                     ("OPeNDAP-Server", "pydap/" + __version__),
                     ("Content-description", "DAP_form"),
                     ("Content-type", "text/html; charset=utf-8"),
-                    ("Content-Length", "8500"),
+                    ("Content-Length", "8101"),
                 ]
             ),
         )