Repository: pydap/pydap
Pull request #421 (2024-12-17 19:12:05)

broaden dmr parser to allow multiple values on single attribute
<!-- Thank you very much for your hard work and contribution! -->

~This is a draft~
The following Pull Request:

- [x] Closes  #406. Correctly parses an attribute with multiple values to generate a list of values.
- [x] Tests are added to demonstrate the new or fixed behavior, and all tests pass
on a local environment.



### example
This is included in the testing

```python
dmr = """
<Dataset name="foo">\n
<Int32 name="x">\n
    <Attribute name="attr" type="Float32">\n
        <Value></Value>\n
        <Value>2</Value>\n
        <Value value="1"/>\n</Attribute>\n
</Int32>\n
</Dataset>
"""
dom_et = DMRParser(dmr).node
dataset = DMRParser(dmr).init_dataset()
variables = get_variables(dom_et)
get_attributes(variables['x']['element'])['attr']
```
the last line returns the list for attribute `'attr'`. All values, excep None, are Float32, associated with variable `'x'` of type int32

```python
[None, 2.0, 1.0]
```


Mikejmnez: @jgallagher59701 ready for review
Mikejmnez: actually - just realized that I am missing case 1 in #421 . Implementing this now. 
Mikejmnez: Done. Added plenty of testing and made sure all 4 cases (and combination of them) described in #406  are covered 

Patch:
diff --git a/src/pydap/parsers/dmr.py b/src/pydap/parsers/dmr.py
index 1651702..dc8a336 100644
--- a/src/pydap/parsers/dmr.py
+++ b/src/pydap/parsers/dmr.py
@@ -85,26 +85,71 @@ def get_dtype(element):
 
 
 def get_atomic_attr(element):
+    """
+    Gets the attribute from an xml.ET element associated with a variable
+    in the dataset. Attributes may be defined in various ways on a DMR,
+    and must be a type that is atomic within the DAP2 and DAP4 spec:
+    see
+    https://opendap.github.io/dap4-specification/DAP4.html#_how_dap4_differs_from_dap2
+
+    Ways that an attribute can be defined according to following cases:
+    1. <Attribute name="name" type={atomic} value="val"/>
+    2. <Attribute name="name" type={atomic}>
+            <Value>val</Value>
+    3. <Attribute name="name" type={atomic}>
+            <Value value="val"/>
+
+    In the general case an attribute may have multiple values, when defined within
+    the scope of the attributes, resulting in an attribute that has a list as values
+    (e.g. range of values). By convention, the order in which the attributes are
+    defined is the way in which these appear on the list.
+
+    Example 1:
+        <Attribute name='range' type='int64'>
+            <Value>-1</Value>
+            <Value>1</Value>
+        </Attribute>
+
+    Then, range = [-1, 1].
+
+    Example 2:
+
+        <Attribute name="range" type="int64" value="-1">
+            <Value>1</Value>
+            <Value value=10/>
+        </Attribute>
+
+    Then range = [-1, 1, 10]
+
+    """
+    # get name of attribute
+    name = element.get("name")
+    # Get type, always a string. If numeric, must be turn into numeric type.
+    _type = element.get("type")
     Float_types = ["Float32", "float", "Float64"]
     Int_types = ["Int16", "Int32", "Int64", "int", "Int8"]
     uInt_types = ["uInt16", "uInt32", "uInt64", "uint", "uInt8", "Char"]
-    name = element.get("name")
-    value = element.find("Value").text
-    if value is None:
-        # This could be because server is TDS.
-        # If value is None still, then data is missing
-        value = element.find("Value").get("value")
-    _type = element.get("type")
-    if _type in dmr_atomic_types and value is not None:
+    # Get values. There may be multiple, and these may be defined in various ways
+    # according to the 3 cases above
+    value = []
+    # Case 1: Inline value definition. May be None.
+    if element.get("value") is not None:
+        value.append(element.get("value"))
+    # Case 2 and 3:
+    value += [
+        val.text if val.text is not None else val.get("value")
+        for val in element.findall("Value")
+    ]
+    if _type in dmr_atomic_types:
         if _type in Float_types:
             # keep float-type of value
-            value = float(value)
+            value = [float(val) if val is not None else val for val in value]
         elif _type in Int_types or _type in uInt_types:
             # keeps integer-type of value
-            value = int(value)
+            value = [int(val) if val is not None else val for val in value]
         else:
             try:
-                value = ast.literal_eval(value)
+                value = [ast.literal_eval(val) for val in value if val is not None]
             except ValueError:
                 # leaves value as string
                 raise Warning(
@@ -114,6 +159,11 @@ def get_atomic_attr(element):
                         name
                     )
                 )
+    if len(value) <= 1:
+        if value != []:
+            value = value[0]
+        else:
+            value = None
     return name, value
 
 
diff --git a/src/pydap/responses/dmr.py b/src/pydap/responses/dmr.py
index 931093e..e866e86 100644
--- a/src/pydap/responses/dmr.py
+++ b/src/pydap/responses/dmr.py
@@ -126,13 +126,23 @@ def _basetype(var, level=0):
         )
     for key, value in var.attributes.items():
         if key != "dims":
-            _type = type(value).__name__
-            yield '{indent}<Attribute name="{name}" type="{type}">\n'.format(
-                indent=(level + 1) * INDENT, name=key, type=_type
-            )
-            yield "{indent}<Value>{val}</Value>\n".format(
-                indent=(level + 2) * INDENT, val=value
-            )
+            if isinstance(value, list):
+                _type = type(value[0]).__name__
+                yield '{indent}<Attribute name="{name}" type="{type}">\n'.format(
+                    indent=(level + 1) * INDENT, name=key, type=_type
+                )
+                for val in value:
+                    yield "{indent}<Value>{val}</Value>\n".format(
+                        indent=(level + 2) * INDENT, val=val
+                    )
+            else:
+                _type = type(value).__name__
+                yield '{indent}<Attribute name="{name}" type="{type}">\n'.format(
+                    indent=(level + 1) * INDENT, name=key, type=_type
+                )
+                yield "{indent}<Value>{val}</Value>\n".format(
+                    indent=(level + 2) * INDENT, val=value
+                )
             yield "{indent}</Attribute>\n".format(indent=(level + 1) * INDENT)
     yield "{indent}</{type}>\n".format(indent=level * INDENT, type=_vartype)
 


Test patch:
diff --git a/src/pydap/tests/data/dmrs/SimpleGroup.dmr b/src/pydap/tests/data/dmrs/SimpleGroup.dmr
index 581ab42..4bbfe31 100644
--- a/src/pydap/tests/data/dmrs/SimpleGroup.dmr
+++ b/src/pydap/tests/data/dmrs/SimpleGroup.dmr
@@ -18,6 +18,10 @@
             <Attribute name="_FillValue" type="float">
                 <Value>inf</Value>
             </Attribute>
+            <Attribute name="ValidRange" type="int">
+                <Value>-10</Value>
+                <Value>100</Value>
+            </Attribute>
         </Float32>
         <Float32 name="Salinity">
             <Dim name="/time"/>
@@ -29,6 +33,10 @@
             <Attribute name="_FillValue" type="float">
                 <Value>nan</Value>
             </Attribute>
+            <Attribute name="ValidRange" type="float">
+                <Value>0.0</Value>
+                <Value>50.0</Value>
+            </Attribute>
         </Float32>
         <Int16 name="Y">
             <Dim name="/SimpleGroup/Y"/>
diff --git a/src/pydap/tests/datasets.py b/src/pydap/tests/datasets.py
index 734a1d8..463be9c 100644
--- a/src/pydap/tests/datasets.py
+++ b/src/pydap/tests/datasets.py
@@ -233,6 +233,7 @@ SimpleGroup.createVariable(
     units="degrees_celsius",
     dimensions=("/time", "/SimpleGroup/Y", "/SimpleGroup/X"),
     _FillValue=np.inf,
+    ValidRange=[-10, 100],
 )
 SimpleGroup.createVariable(
     name="/SimpleGroup/Salinity",
@@ -240,6 +241,7 @@ SimpleGroup.createVariable(
     units="psu",
     dimensions=("/time", "/SimpleGroup/Y", "/SimpleGroup/X"),
     _FillValue=np.nan,
+    ValidRange=[0.0, 50.0],
 )
 SimpleGroup.createVariable(
     name="/SimpleGroup/Y", data=np.arange(4, dtype="i2"), dimensions=("/SimpleGroup/Y",)
diff --git a/src/pydap/tests/test_parsers_dmr.py b/src/pydap/tests/test_parsers_dmr.py
index 9ec3a4d..b303ec5 100644
--- a/src/pydap/tests/test_parsers_dmr.py
+++ b/src/pydap/tests/test_parsers_dmr.py
@@ -266,6 +266,31 @@ class TestAttrsTypesDMRParser(unittest.TestCase):
         ds = dmr_to_dataset(dmr)
         assert isinstance(ds["x"].attributes["attr"], float)
 
+    def test_multiple_entries(self):
+        dmr = """<Dataset name="foo">\n    <Int32 name="x">\n
+        <Attribute name="attr" type="Float32">\n
+                    <Value>-1</Value>\n        <Value value="1"/>\n</Attribute>\n
+                        </Int32>\n</Dataset>"""
+
+        dmr2 = """<Dataset name="foo">\n    <Int32 name="x">\n
+        <Attribute name="attr" type="Float32">\n
+                    <Value value="-1"/>\n        <Value>1</Value>\n</Attribute>\n
+                        </Int32>\n</Dataset>"""
+
+        ds = dmr_to_dataset(dmr)
+        ds2 = dmr_to_dataset(dmr2)
+        assert ds["x"].attributes["attr"] == [-1.0, 1.0]
+        assert ds2["x"].attributes["attr"] == [-1.0, 1.0]
+
+    def test_multiple_entries2(self):
+        dmr = """<Dataset name="foo">\n    <Int32 name="x">\n
+        <Attribute name="attr" type="Float32" value="100">\n
+                    <Value></Value>\n        <Value>2</Value>\n
+                            <Value value="1"/>\n</Attribute>\n
+                        </Int32>\n</Dataset>"""
+        ds = dmr_to_dataset(dmr)
+        assert ds["x"].attributes["attr"] == [100, None, 2.0, 1.0]
+
     def test_string(self):
         dmr = """<Dataset name="foo">\n    <String name="bears">\n
                 <Attribute name="attr" type="Float32">\n
@@ -274,3 +299,13 @@ class TestAttrsTypesDMRParser(unittest.TestCase):
         ds = dmr_to_dataset(dmr)
         assert "bears" in ds.variables()
         assert ds["bears"].dtype == "S128"
+
+    def test_attr_InlineValue(self):
+        dmr = """
+        <Dataset name="foo">\n
+        <String name="bears">\n
+            <Attribute name="attr" type="Float32" value="100"/>\n
+        </String>\n
+        </Dataset>"""
+        ds = dmr_to_dataset(dmr)
+        assert ds["bears"].attributes["attr"] == 100.0
diff --git a/src/pydap/tests/test_responses_dmr.py b/src/pydap/tests/test_responses_dmr.py
index 54d6ee6..b9aab66 100644
--- a/src/pydap/tests/test_responses_dmr.py
+++ b/src/pydap/tests/test_responses_dmr.py
@@ -58,7 +58,7 @@ class TestDMRResponseGroup(unittest.TestCase):
                         "Access-Control-Allow-Headers",
                         "Origin, X-Requested-With, Content-Type",
                     ),
-                    ("Content-Length", "1977"),
+                    ("Content-Length", "2276"),
                 ]
             ),
         )
diff --git a/src/pydap/tests/test_responses_html.py b/src/pydap/tests/test_responses_html.py
index 6b147c7..050ccbf 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", "8101"),
+                    ("Content-Length", "8224"),
                 ]
             ),
         )