Merge pull request #108 from csiro-coasts/normalize-depth-optional-pa…

…rameters

Make `normalize_depth_variables()` parameters optional

docs/releases/development.rst

@@ -20,4 +20,10 @@ Next release (in development)
   (:pr:`103`).
 * Add :meth:`emsarray.plot.add_landmarks()`
   and `landmarks` parameter to :meth:`Convention.plot` and related functions.
-  (:pr:`105`).
+  (:pr:`107`).
+* Make the `positive_down` and `deep_to_shallow` parameters optional
+  for :func:`~emsarray.operations.depth.normalize_depth_variables`.
+  If not supplied, that feature of the depth variable is not normalized.
+  This is a breaking change if you previously relied
+  on the default value of `True` for these parameters.
+  (:pr:`108`).

src/emsarray/conventions/_base.py

@@ -1716,7 +1716,10 @@ def ocean_floor(self) -> xarray.Dataset:
             non_spatial_variables=[self.get_time_name()])
 
     def normalize_depth_variables(
-        self, positive_down: bool = True, deep_to_shallow: bool = True,
+        self,
+        *,
+        positive_down: Optional[bool] = None,
+        deep_to_shallow: Optional[bool] = None,
     ) -> xarray.Dataset:
         """An alias for :func:`emsarray.operations.depth.normalize_depth_variables`"""
         return depth.normalize_depth_variables(

src/emsarray/operations/depth.py

@@ -197,39 +197,44 @@ def normalize_depth_variables(
     dataset: xarray.Dataset,
     depth_variables: List[Hashable],
     *,
-    positive_down: bool = True,
-    deep_to_shallow: bool = True,
+    positive_down: Optional[bool] = None,
+    deep_to_shallow: Optional[bool] = None,
 ) -> xarray.Dataset:
     """
     Some datasets represent depth as a positive variable, some as negative.
     Some datasets sort depth layers from deepest to most shallow, others
     from shallow to deep. :func:`normalize_depth_variables` will return
     a new dataset with the depth variables normalized.
 
-    The default behaviour is for positive values to indicate deeper depths
-    (indicated via the variable attribute ``positive: "down"``),
-    and for the layers to be ordered deep to shallow.
-    This behaviour can be modified using the parameters
-    ``positive_down`` and ``deep_to_shallow`` respectively.
+    All depth variables should have a ``positive: "up"`` or ``positive: "down"`` attribute.
+    If this attribute is missing,
+    a warning is generated and
+    a value is determined by examining the coordinate values.
 
     Parameters
     ----------
-    dataset
+    dataset : xarray.Dataset
         The dataset to normalize
-    depth_variables
+    depth_variables : list of Hashable
         The names of the depth coordinate variables.
         This should be the names of the variables, not the dimensions,
         for datasets where these differ.
-    positive_down
-        When true (the default), positive values will indicate depth below the
-        surface. When false, negative values indicate depth below the surface.
-    deep_to_shallow
-        When true (the default), the layers are ordered such that deeper layers
-        have lower indices.
+    positive_down : bool, optional
+        If True, positive values will indicate depth below the surface.
+        If False, negative values indicate depth below the surface.
+        If None, this attribute of the depth coordinate is left unmodified.
+    deep_to_shallow : bool, optional
+        If True, the layers are ordered such that deeper layers have lower indices.
+        If False, the layers are ordered such that deeper layers have higher indices.
+        If None, this attribute of the depth coordinate is left unmodified.
+
+    Returns
+    -------
+    xarray.Dataset
+        A copy of the dataset with the depth variables normalized.
 
     See Also
     --------
-
     :meth:`.Convention.normalize_depth_variables`
     :meth:`.Convention.get_all_depth_names`
     """
@@ -244,23 +249,29 @@ def normalize_depth_variables(
         dimension = variable.dims[0]
 
         new_variable = new_dataset[name]
-        new_variable.attrs['positive'] = 'down' if positive_down else 'up'
-
-        positive_attr = variable.attrs.get('positive')
-        if positive_attr is None:
-            # This is a _depth_ variable. If there are more values >0 than <0,
-            # positive is probably down.
+        if positive_down is not None:
+            new_variable.attrs['positive'] = 'down' if positive_down else 'up'
+
+        if 'positive' in variable.attrs:
+            positive_attr = variable.attrs.get('positive')
+            data_positive_down = (positive_attr == 'down')
+        else:
+            # No positive attribute set.
+            # This is a violation of the CF conventions,
+            # however it is a very common violation and we can make a good guess.
+            # This is a _depth_ variable.
+            # If there are more values >0 than <0, positive is probably down.
             total_values = len(variable.values)
             positive_values = len(variable.values[variable.values > 0])
-            positive_attr = 'down' if positive_values > total_values / 2 else 'up'
+            data_positive_down = positive_values > (total_values / 2)
 
             warnings.warn(
                 f"Depth variable {name!r} had no 'positive' attribute, "
-                f"guessing `positive: {positive_attr!r}`",
+                f"guessing `positive: {'down' if data_positive_down else 'up'!r}`",
                 stacklevel=2)
 
-        # Reverse the polarity
-        if (positive_attr == 'down') != positive_down:
+        if positive_down is not None and data_positive_down != positive_down:
+            # Reverse the polarity
             new_values = -1 * new_variable.values
             if name == dimension:
                 new_dataset = new_dataset.assign_coords({name: new_values})
@@ -273,15 +284,34 @@ def normalize_depth_variables(
                 })
                 new_variable = new_dataset[name]
 
-        # Check if the existing data goes from deep to shallow, correcting for
-        # the positive_down we just adjusted above. This assumes that depth
-        # data are monotonic across all values. If this is not the case,
-        # good luck.
-        d1, d2 = new_variable.values[0:2]
-        data_deep_to_shallow = (d1 > d2) == positive_down
+            try:
+                bounds_name = new_variable.attrs['bounds']
+                bounds_variable = new_dataset[bounds_name]
+            except KeyError:
+                pass
+            else:
+                new_dataset = new_dataset.assign({
+                    bounds_name: (
+                        bounds_variable.dims,
+                        -1 * bounds_variable.values,
+                        bounds_variable.attrs,
+                        bounds_variable.encoding,
+                    ),
+                })
+
+            # Update this so the deep-to-shallow normalization can use it
+            data_positive_down = positive_down
+
+        if deep_to_shallow is not None:
+            # Check if the existing data goes from deep to shallow, correcting for
+            # the positive_down we just adjusted above. This assumes that depth
+            # data are monotonic across all values. If this is not the case,
+            # good luck.
+            d1, d2 = new_variable.values[0:2]
+            data_deep_to_shallow = (d1 > d2) == data_positive_down
 
-        # Flip the order of the coordinate
-        if data_deep_to_shallow != deep_to_shallow:
-            new_dataset = new_dataset.isel({dimension: numpy.s_[::-1]})
+            # Flip the order of the coordinate
+            if data_deep_to_shallow != deep_to_shallow:
+                new_dataset = new_dataset.isel({dimension: numpy.s_[::-1]})
 
     return new_dataset

tests/operations/depth/test_normalize_depth_variables.py

@@ -1,3 +1,5 @@
+from typing import Optional
+
 import numpy
 import pytest
 import xarray
@@ -7,7 +9,7 @@
 
 
 @pytest.mark.parametrize(
-    "input_depths,input_positive,input_deep_to_shallow", [
+    ["input_depths", "input_positive", "input_deep_to_shallow"], [
         ([-1, +0, +1, +2, +3, +4], 'down', False),
         ([+4, +3, +2, +1, +0, -1], 'down', True),
         ([+1, +0, -1, -2, -3, -4], 'up', False),
@@ -16,31 +18,39 @@
 )
 @pytest.mark.parametrize("set_positive", [True, False])
 @pytest.mark.parametrize(
-    ["expected", "positive_down", "deep_to_shallow"], [
-        ([+4, +3, +2, +1, +0, -1], True, True),
-        ([-1, +0, +1, +2, +3, +4], True, False),
-        ([-4, -3, -2, -1, +0, +1], False, True),
-        ([+1, +0, -1, -2, -3, -4], False, False),
+    ["positive_down", "deep_to_shallow"], [
+        (None, None),
+        (None, True),
+        (None, False),
+        (True, None),
+        (True, True),
+        (True, False),
+        (False, None),
+        (False, True),
+        (False, False),
     ],
 )
 def test_normalize_depth_variable(
-    input_depths: numpy.ndarray, input_positive: str, input_deep_to_shallow: bool,
+    input_depths: list[int],
+    input_positive: str,
+    input_deep_to_shallow: bool,
     set_positive: bool,
-    expected: numpy.ndarray, positive_down: bool, deep_to_shallow: bool,
+    positive_down: Optional[bool],
+    deep_to_shallow: Optional[bool],
     recwarn,
 ):
-    input_depths = input_depths[:]
     # Some datasets have a coordinate with the same dimension name
+    positive_attr = {'positive': input_positive} if set_positive else {}
     depth_coord = xarray.DataArray(
         data=numpy.array(input_depths),
         dims=['depth_coord'],
-        attrs={'positive': input_positive if set_positive else None, 'foo': 'bar'},
+        attrs={**positive_attr, 'foo': 'bar'},
     )
     # Some dimensions have different coordinate and dimension names
     depth_name = xarray.DataArray(
         data=numpy.array(input_depths),
         dims=['depth_dimension'],
-        attrs={'positive': input_positive if set_positive else None, 'foo': 'bar'},
+        attrs={**positive_attr, 'foo': 'bar'},
     )
     values = numpy.arange(4 * 6 * 4).reshape(4, 6, 4)
     dataset = xarray.Dataset(
@@ -57,32 +67,45 @@ def test_normalize_depth_variable(
     )
 
     out = normalize_depth_variables(
-        dataset, ['depth_coord', 'depth_name'],
-        positive_down=positive_down, deep_to_shallow=deep_to_shallow,
+        dataset,
+        ['depth_coord', 'depth_name'],
+        positive_down=positive_down,
+        deep_to_shallow=deep_to_shallow,
     )
 
+    expected_attrs = {'foo': 'bar'}
+    if positive_down is None:
+        if set_positive:
+            expected_attrs['positive'] = input_positive
+    elif positive_down:
+        expected_attrs['positive'] = 'down'
+    else:
+        expected_attrs['positive'] = 'up'
+
     # Check that the values are reordered along the depth axis if required
-    expected_values = (
-        values[:, :, :] if (input_deep_to_shallow == deep_to_shallow)
-        else values[:, ::-1, :])
+    expected_values = values.copy()
+    if deep_to_shallow is not None and input_deep_to_shallow != deep_to_shallow:
+        expected_values = expected_values[:, ::-1, :]
     assert_equal(out['values_coord'].values, expected_values)
     assert_equal(out['values_dimension'].values, expected_values)
 
     # Check that attributes on the depth coordinate were not clobbered
-    assert out['depth_coord'].attrs == {
-        'positive': 'down' if positive_down else 'up',
-        'foo': 'bar',
-    }
-    assert out['depth_name'].attrs == {
-        'positive': 'down' if positive_down else 'up',
-        'foo': 'bar',
-    }
+    assert out['depth_coord'].attrs == expected_attrs
+    assert out['depth_name'].attrs == expected_attrs
 
     # Check the depth values are as expected
-    assert_equal(out['depth_coord'].values, expected)
+    # The depths should be similar to the inputs,
+    # possibly reversed and possibly negated depending on the input parameters.
+    expected_depths = numpy.array(input_depths)
+    if positive_down is not None and input_positive != expected_attrs['positive']:
+        expected_depths = -expected_depths
+    if deep_to_shallow is not None and input_deep_to_shallow != deep_to_shallow:
+        expected_depths = expected_depths[::-1]
+
+    assert_equal(out['depth_coord'].values, expected_depths)
     assert out['depth_coord'].dims == ('depth_coord',)
 
-    assert_equal(out['depth_name'].values, expected)
+    assert_equal(out['depth_name'].values, expected_depths)
     assert out['depth_name'].dims == ('depth_dimension',)
 
     assert out.dims['depth_coord'] == 6