Bugfix for padding (#597)

* Trigger bug in test and add fix

* switching to fix sgrid bug

* Fixed test

* add whats new entry

* Update whats-new.rst

doc/whats-new.rst

@@ -15,9 +15,9 @@ New Features
   addition of SGRID conventions (:issue:`109`, :pull:`559`).
   By `Jack Atkinson <https://github.com/jatkinson1000>`_.
 
+
 Breaking Changes
 ~~~~~~~~~~~~~~~~
-
 - All computation methods on the :py:class:`xgcm.Axis` class have been removed, in favour of using the corresponding
   methods on the :py:class:`xgcm.Grid` object. The :py:class:`xgcm.Axis` class has also been removed from public API.
   (:issue:`405`, :pull:`557`).
@@ -39,7 +39,8 @@ Documentation
 
 Bugfixes
 ~~~~~~~~
-
+- Fix bug in :py:meth:`xgcm.padding._maybe_rename_grid_positions` where dimensions were assumed to have coordinate values leading to errors with ECCO data. (:issue:`531`, :issue:`595`, :pull:`597`).
+  By `Julius Busecke <https://github.com/jbusecke>`_.
 
 v0.8.1 (2022/11/22)
 -------------------

xgcm/padding.py

@@ -25,7 +25,7 @@ def _maybe_rename_grid_positions(grid, arr_source, arr_target):
     rename_dict = {}
     for di in arr_target.dims:
         # in case the dimension is already in the source, do nothing.
-        if di not in arr_source:
+        if di not in arr_source.dims:
             # find associated axis
             for axname in grid.axes:
                 all_positions = grid.axes[axname].coords.values()

xgcm/test/test_faceconnections.py

@@ -3,6 +3,7 @@
 import xarray as xr
 
 from xgcm.grid import Grid
+from xgcm.metadata_parsers import parse_comodo
 
 
 @pytest.fixture(scope="module")
@@ -217,9 +218,28 @@ def test_vector_connected_grid_x_to_y(ds, ds_face_connections_x_to_y, boundary):
     np.testing.assert_allclose(v_out.data, 1)
 
 
-def test_vector_diff_interp_connected_grid_x_to_y(ds, ds_face_connections_x_to_y):
-    # simplest scenario with one face connection
-    grid = Grid(ds, face_connections=ds_face_connections_x_to_y)
+@pytest.mark.parametrize("no_coords", [True, False])
+def test_vector_diff_interp_connected_grid_x_to_y(
+    ds, ds_face_connections_x_to_y, no_coords
+):
+    # TODO: this is not elegant. This test should perhaps not use metadata parsing.
+    # Instead we can use a dataset_factory fixture to create the dataset, and input kwargs with different input options (e.g. no coords)
+    if no_coords:
+        """Trigger error in https://github.com/xgcm/xgcm/issues/595 and https://github.com/xgcm/xgcm/issues/531 by removing coords from dataset."""
+        # parse comodo metadata before removing coords
+        ds, comodo_grid_kwargs = parse_comodo(ds)
+        ds = ds.drop_vars(
+            [di for di in ds.dims if di != "face"]
+        )  # need to retain the face dimension coordinates here. I wonder if this should actually work without coords?
+        grid = Grid(
+            ds,
+            **comodo_grid_kwargs,
+            face_connections=ds_face_connections_x_to_y,
+            autoparse_metadata=False
+        )
+    else:
+        # simplest scenario with one face connection
+        grid = Grid(ds, face_connections=ds_face_connections_x_to_y)
 
     # interp u and v to cell center
     vector_center = grid.interp_2d_vector(