raise error when select on bad patch coord (#556)

d-chambers · web-flow · commit 9b94e78fedd3 · 2025-09-27T14:50:35.000+01:00
* raise error when select on bad patch coord

* fix select typo

* added coord manager select tests

* select on non-dim coords

* tests for selecting non-dimensional coord

* review
diff --git a/dascore/core/coordmanager.py b/dascore/core/coordmanager.py
@@ -115,22 +115,31 @@ def _get_indexers_and_new_coords_dict(
 ):
     """Get reductions for each dimension."""
     dim_reductions = {x: slice(None, None) for x in cm.dims}
-    dimap = cm.dim_map
     new_coords = dict(cm._get_dim_array_dict(keep_coord=True))
     for coord_name, vals in kwargs.items():
-        # this is not a selectable coord, just skip.
-        if coord_name not in cm.coord_map or not len(cm.dim_map[coord_name]):
-            continue
+        # All coordinates should exist in coord_map (filtered by
+        # _get_single_dim_kwarg_list)
+        assert coord_name in cm.coord_map
         coord = cm.coord_map[coord_name]
+        coord_dims = cm.dim_map[coord_name]
         _ensure_1d_coord(coord, coord_name)
-        dim_name = dimap[coord_name][0]
+        # Handle non-dimensional coordinates (not tied to any dimension)
+        if not len(coord_dims):
+            # Apply operation directly to the non-dimensional coordinate
+            method = getattr(coord, operation)
+            new_coord, _ = method(vals, relative=relative, samples=samples)
+            # Update only this coordinate in new_coords, don't affect array indexing
+            new_coords[coord_name] = (coord_dims, new_coord)
+            continue
+        # Handle dimensional coordinates (tied to exactly one dimension)
+        dim_name = coord_dims[0]
         # different logic if we are using indices or values
         method = getattr(coord, operation)
         new_coord, reductions = method(vals, relative=relative, samples=samples)
         # this handles the case of out-of-bound selections.
         # These should be converted to degenerate coords.
         dim_reductions[dim_name] = reductions
-        new_coords[coord_name] = (dimap[coord_name], new_coord)
+        new_coords[coord_name] = (coord_dims, new_coord)
         # update other coords affected by change.
         _indirect_coord_updates(cm, dim_name, coord_name, reductions, new_coords)
     indexers = tuple(dim_reductions[x] for x in cm.dims)
diff --git a/dascore/proc/coords.py b/dascore/proc/coords.py
@@ -10,7 +10,12 @@
 
 from dascore.constants import PatchType, select_values_description
 from dascore.core.coords import BaseCoord
-from dascore.exceptions import CoordError, ParameterError, PatchError
+from dascore.exceptions import (
+    CoordError,
+    ParameterError,
+    PatchCoordinateError,
+    PatchError,
+)
 from dascore.utils.docs import compose_docstring
 from dascore.utils.misc import get_parent_code_name, iterate
 from dascore.utils.patch import patch_function
@@ -487,7 +492,7 @@ def select(
     >>>
     >>> # Select only specific values along a dimension
     >>> distance = patch.get_array("distance")
-    >>> new_distance_3 = patch.select(distace=distance[1::2])
+    >>> new_distance_3 = patch.select(distance=distance[1::2])
 
     Notes
     -----
@@ -497,14 +502,23 @@ def select(
       See [`Patch.order`](`dascore.Patch.order`).
 
     """
+    # Check for and raise on invalid kwargs.
+    if invalid_coords := set(kwargs) - set(patch.coords.coord_map):
+        invalid_list = sorted(invalid_coords)
+        valid_list = sorted(patch.coords.coord_map)
+        msg = (
+            f"Coordinate(s) {invalid_list} not found in patch coordinates: {valid_list}"
+        )
+        raise PatchCoordinateError(msg)
+
     new_coords, data = patch.coords.select(
         **kwargs,
         array=patch.data,
         relative=relative,
         samples=samples,
     )
-    # no slicing was performed, just return original.
-    if data.shape == patch.data.shape:
+    # no slicing was performed, just return original if coordinates also unchanged.
+    if data.shape == patch.data.shape and new_coords == patch.coords:
         return patch
     if copy:
         data = data.copy()
diff --git a/dascore/proc/hampel.py b/dascore/proc/hampel.py
@@ -96,16 +96,12 @@ def hampel_filter(
     >>> data[10, 5] = 10  # Add a large spike
     >>> patch = patch.update(data=data)
     >>>
-    >>> # Apply hampel filter along time dimension with 1.0 unit window
-    >>> filtered = patch.hampel_filter(time=1.0, threshold=3.5)
+    >>> # Apply hampel filter along time dimension with 0.2 unit window
+    >>> filtered = patch.hampel_filter(time=0.2, threshold=3.5)
     >>> assert filtered.data.shape == patch.data.shape
     >>> # The spike should be reduced
     >>> assert abs(filtered.data[10, 5]) < abs(patch.data[10, 5])
     >>>
-    >>> # Apply filter with a lower threshold for more aggressive filtering
-    >>> filtered_aggressive = patch.hampel_filter(time=1.0, threshold=2.0)
-    >>> assert isinstance(filtered_aggressive, dc.Patch)
-    >>>
     >>> # Apply filter along multiple dimensions:
     >>> filtered_2d = patch.hampel_filter(time=1.0, distance=5.0, threshold=3.5)
     >>> assert filtered_2d.data.shape == patch.data.shape
@@ -115,10 +111,6 @@ def hampel_filter(
     ...     time=3, distance=3, samples=True, threshold=3.5
     ... )
     >>>
-    >>> # Use separable filtering for faster processing (approximation)
-    >>> filtered_fast = patch.hampel_filter(
-    ...     time=1.0, distance=5.0, threshold=3.5, separable=True
-    ... )
     """
     if threshold <= 0 or not np.isfinite(threshold):
         msg = "hampel_filter threshold must be finite and greater than zero"
diff --git a/tests/test_core/test_coordmanager.py b/tests/test_core/test_coordmanager.py
@@ -621,6 +621,116 @@ def test_select_non_dim_coord(self, cm_basic):
         out, _ = new_cm.select(new_dim=(1, 20))
         assert new_cm == out
 
+    def test_select_non_dim_coord_shortens_coordinate(self, cm_basic):
+        """Test that selecting non-dimensional coords shortens only that coordinate."""
+        # Add a non-dimensional coordinate with numeric values
+        quality_scores = np.array([0.1, 0.5, 0.9, 0.2, 0.8, 0.3, 0.7])
+        new_cm = cm_basic.update(quality=(None, quality_scores))
+        # Select subset of quality scores using array indexing
+        selected_indices = np.array([1, 3, 5])  # Select indices 1, 3, 5
+        out, _ = new_cm.select(quality=selected_indices, samples=True)
+        # Quality coordinate should be shortened
+        expected_quality = quality_scores[selected_indices]
+        assert np.array_equal(out.get_array("quality"), expected_quality)
+        # Dimensional coordinates should be unchanged
+        assert cm_basic.shape == out.shape
+        assert np.array_equal(out.get_array("time"), new_cm.get_array("time"))
+        assert np.array_equal(out.get_array("distance"), new_cm.get_array("distance"))
+
+    def test_select_non_dim_coord_with_boolean_mask(self, cm_basic):
+        """Test selecting non-dimensional coordinates using boolean arrays."""
+        # Add a non-dimensional coordinate
+        values = np.array([10, 20, 30, 40, 50, 60, 70])
+        new_cm = cm_basic.update(sensor_values=(None, values))
+        # Create boolean mask
+        mask = values > 35  # Should select [40, 50, 60, 70]
+        out, _ = new_cm.select(sensor_values=mask)
+        # Only the non-dimensional coordinate should be affected
+        expected_values = values[mask]
+        assert np.array_equal(out.get_array("sensor_values"), expected_values)
+        # Dimensional coordinates should remain unchanged
+        for coord in set(cm_basic.coord_map) - {"sensor_values"}:
+            assert cm_basic.get_coord(coord) == new_cm.get_coord(coord)
+
+    def test_select_multi_dim_coord_raises(self, cm_multidim):
+        """
+        Coords that are associated with more than one dim cannot be selected
+        because it could ruin the squareness of the patch.
+        """
+        # Non-dim coord associated with one dimension should work.
+        lat = cm_multidim.get_array("latitude")
+        lat_mean = np.mean(lat)
+        out, _ = cm_multidim.select(latitude=(..., lat_mean))
+        assert isinstance(out, dc.CoordManager)
+        # Multi-dim coord should raise CoordError
+        msg = "Only 1 dimensional coordinates"
+        with pytest.raises(CoordError, match=msg):
+            cm_multidim.select(quality=(1, 20))
+
+    def test_select_coord_tied_to_dimension_affects_others(self, cm_multidim):
+        """
+        Test that selecting a coord tied to a dimension affects other coords
+        on that dim.
+        """
+        # cm_multidim should have coordinates that share dimensions
+        # Get a coordinate that's tied to a dimension and has other coords sharing
+        # that dim
+        lat = cm_multidim.get_array("latitude")
+        lat_mean = np.mean(lat)
+        out, _ = cm_multidim.select(latitude=(..., lat_mean))
+        # Check that the new lat is what we expect.
+        new_lat = out.get_array("latitude")
+        expected = lat[lat <= lat_mean]
+        assert np.array_equal(new_lat, expected)
+        # And the other coord associated with that dimension have the same len.
+        for name, coord in out.coord_map.items():
+            coord_dims = out.dim_map[name]
+            # Skip coords not tied to distance dimension
+            if "distance" not in coord_dims:
+                continue
+            axis = coord_dims.index("distance")
+            assert coord.shape[axis] == len(new_lat)
+
+    def test_select_nonexistent_coordinate_ignores_gracefully(self, cm_basic):
+        """Test that selecting on a non-existent coordinate is ignored gracefully."""
+        # This tests line 122 in _get_indexers_and_new_coords_dict
+        original_shape = cm_basic.shape
+        out, _ = cm_basic.select(nonexistent_coord=(1, 10))
+
+        # Should return unchanged coordinate manager
+        assert out == cm_basic
+        assert out.shape == original_shape
+
+        # Should work with multiple nonexistent coordinates
+        out2, _ = cm_basic.select(
+            fake_coord1=(1, 2), fake_coord2=slice(0, 5), another_fake=(10, 20)
+        )
+        assert out2 == cm_basic
+        assert out2.shape == original_shape
+
+    def test_select_mix_valid_invalid_coordinates(self, cm_basic):
+        """Test selecting with mix of valid and invalid coordinate names."""
+        # This also exercises line 122 but with mixed scenarios
+        time_vals = cm_basic.get_array("time")
+        subset_time = (time_vals[1], time_vals[-2])
+
+        out, _ = cm_basic.select(
+            time=subset_time,  # valid coordinate
+            nonexistent=(1, 10),  # invalid coordinate - should be ignored
+            fake_dim=slice(0, 5),  # another invalid coordinate
+        )
+
+        # Only the valid coordinate selection should have been applied
+        assert (
+            out.shape[cm_basic.get_axis("time")]
+            < cm_basic.shape[cm_basic.get_axis("time")]
+        )
+        # Distance should be unchanged since it wasn't selected
+        assert (
+            out.shape[cm_basic.get_axis("distance")]
+            == cm_basic.shape[cm_basic.get_axis("distance")]
+        )
+
 
 class TestOrder:
     """Tests for ordering coordinate managers."""
diff --git a/tests/test_proc/test_proc_coords.py b/tests/test_proc/test_proc_coords.py
