dascore/proc/basic.py (1)

758-791: Normalize dims, no-op fast-path, and clearer handling when dims is empty

• Deduplicate dims to avoid double-flipping a dimension.
• Fast-return when no dims and flip_coords is False to avoid unnecessary history churn.
• Guard np.flip when dims is empty (axis=()) to be explicit.

 @patch_function()
 def flip(patch, *dims, flip_coords=True):
@@
-    axes = tuple(patch.get_axis(name) for name in dims)
-    data = np.flip(patch.data, axis=axes)
-    coords = patch.coords
-    if flip_coords:
-        coords = coords.flip(*dims)
-    return patch.new(data=data, coords=coords)
+    # drop duplicate dims, preserve first-seen order
+    dims = tuple(dict.fromkeys(dims))
+    if not dims and not flip_coords:
+        return patch  # no-op
+    axes = tuple(patch.get_axis(name) for name in dims)
+    data = np.flip(patch.data, axis=axes) if dims else patch.data
+    coords = patch.coords.flip(*dims) if flip_coords and dims else patch.coords
+    return patch.new(data=data, coords=coords)

tests/test_proc/test_basic.py (1)

777-813: Augment flip tests to assert data reversal and double-flip identity

Add assertions that data actually reversed, and that double flip restores the original; also verify data reversal when flip_coords=False.

 class TestFlip:
@@
     def test_no_coords(self, random_patch):
         """Ensure coord doesn't change with flip_coords=False"""
         flipped_patch = random_patch.flip("time", flip_coords=False)
         assert flipped_patch.get_coord("time") == random_patch.get_coord("time")
+
+    def test_flip_reverses_data_time(self, random_patch):
+        axis = random_patch.get_axis("time")
+        out = random_patch.flip("time")
+        expected = np.flip(random_patch.data, axis=axis)
+        assert np.array_equal(out.data, expected)
+
+    def test_double_flip_restores(self, random_patch):
+        out = random_patch.flip("time").flip("time")
+        assert random_patch.equals(out)
+
+    def test_flip_coords_false_reverses_data_only(self, random_patch):
+        axis = random_patch.get_axis("time")
+        out = random_patch.flip("time", flip_coords=False)
+        assert np.array_equal(out.data, np.flip(random_patch.data, axis=axis))
+        assert out.get_coord("time") == random_patch.get_coord("time")

dascore/core/coordmanager.py (1)

1098-1120: Nit: avoid shadowing dims variable inside the loop

dims is used as both the method’s argument and a local name for associated coord dims. Rename the local to assoc_dims for clarity.

-                dims = dim_map[associated]
-                axis = dims.index(name)
-                out[associated] = (dims, _flip_coord(out[associated], axis))
+                assoc_dims = dim_map[associated]
+                axis = assoc_dims.index(name)
+                out[associated] = (assoc_dims, _flip_coord(out[associated], axis))

tests/test_core/test_coordmanager.py (5)

1355-1373: Assert immutability (flip should not mutate original CoordManager)

Add a quick identity check to ensure a new instance is returned.

         flipped_cm = cm.flip("time")
 
         # The flipped coordinate should be reversed
         flipped_time = flipped_cm.get_array("time")
         assert np.array_equal(flipped_time, original_time[::-1])
 
         # Other coordinates should remain unchanged
         assert np.array_equal(
             flipped_cm.get_array("distance"), cm.get_array("distance")
         )
 
         # Shape and dimensions should remain the same
         assert flipped_cm.shape == cm.shape
         assert flipped_cm.dims == cm.dims
+        # Should not mutate original object
+        assert flipped_cm is not cm

---

1393-1412: Also verify 2D associated coord flips along the targeted axis

When flipping distance, 2D coords that include distance (e.g., quality with dims ("time","distance")) should reverse along that axis. Adding this assertion hardens coverage and would catch regressions from implementation refactors.

Note: Impl in dascore/core/coordmanager.py flips associated coords via dim_to_coord_map; a test on quality guards against variable shadowing in that method.

         # Associated coordinates (like latitude) should also be flipped
         flipped_latitude = flipped_cm.get_array("latitude")
         assert np.array_equal(flipped_latitude, original_latitude[::-1])
 
         # Time should remain unchanged
         assert np.array_equal(flipped_cm.get_array("time"), cm.get_array("time"))
+
+        # 2D associated coordinate (quality) should be flipped along distance axis
+        q_before = cm.coord_map["quality"].values
+        q_after = flipped_cm.coord_map["quality"].values
+        assert np.array_equal(q_after, q_before[:, ::-1])

---

1374-1392: Strengthen multi-dim flip test by asserting 2D coord flips on both axes

This ensures quality reverses on time and distance when both dims are flipped.

         flipped_cm = cm.flip("time", "distance")
 
         # Both coordinates should be reversed
         flipped_time = flipped_cm.get_array("time")
         flipped_distance = flipped_cm.get_array("distance")
 
         assert np.array_equal(flipped_time, original_time[::-1])
         assert np.array_equal(flipped_distance, original_distance[::-1])
 
         # Shape and dimensions should remain the same
         assert flipped_cm.shape == cm.shape
         assert flipped_cm.dims == cm.dims
+
+        # And 2D coord should reverse on both axes
+        q0 = cm.coord_map["quality"].values
+        q1 = flipped_cm.coord_map["quality"].values
+        assert np.array_equal(q1, q0[::-1, ::-1])

---

1420-1426: Clarify semantics for flipping non‑dim 1D coords (e.g., 'latitude')

Today, flip allows flipping any 1D coord, not just dims. Flipping a non‑dim coord alone will reverse it without touching its associated dim, which changes value‑to‑index alignment. If this is intended, make it explicit with a test; if not, consider raising CoordError for non‑dim inputs.

+    def test_flip_non_dim_coord_semantics(self, cm_multidim):
+        """Flipping a 1D non-dim coord reverses it without altering dims."""
+        lat0 = cm_multidim.get_array("latitude")
+        dist0 = cm_multidim.get_array("distance")
+        out = cm_multidim.flip("latitude")
+        assert np.array_equal(out.get_array("latitude"), lat0[::-1])
+        assert np.array_equal(out.get_array("distance"), dist0)

Alternative: if undesired, assert it raises and adjust implementation accordingly.

---

1464-1477: Add a companion “flip all dims” test for associated coords (multidim fixture)

The cm_basic case can’t validate associated coords. Consider an extra test using cm_multidim to confirm propagation when flipping all dims.

+    def test_flip_all_dimensions_propagates_to_associated(self, cm_multidim):
+        out = cm_multidim.flip(*cm_multidim.dims)
+        # latitude shares 'distance' -> should reverse
+        lat0 = cm_multidim.get_array("latitude")
+        assert np.array_equal(out.get_array("latitude"), lat0[::-1])
+        # quality shares both -> reverse on both axes
+        q0 = cm_multidim.coord_map["quality"].values
+        q1 = out.coord_map["quality"].values
+        assert np.array_equal(q1, q0[::-1, ::-1])

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro