Repository: xgcm/xgcm (python)

Issue:
[BUG] Grid creation for coordinateless face dimension fails
I am working on #475 and noticed a bug

```python
import xarray as xr
import numpy as np

da = xr.DataArray(np.ones([4,4,13]), dims=['i','j','face']) * xr.DataArray(np.arange(13), dims=['face'])
ds = da.to_dataset(name='data')

# ECCO connectivity
face_connections = {'face':
                    {0: {'X':  ((12, 'Y', False), (3, 'X', False)),
                         'Y':  (None,             (1, 'Y', False))},
                     1: {'X':  ((11, 'Y', False), (4, 'X', False)),
                         'Y':  ((0, 'Y', False),  (2, 'Y', False))},
                     2: {'X':  ((10, 'Y', False), (5, 'X', False)),
                         'Y':  ((1, 'Y', False),  (6, 'X', False))},
                     3: {'X':  ((0, 'X', False),  (9, 'Y', False)),
                         'Y':  (None,             (4, 'Y', False))},
                     4: {'X':  ((1, 'X', False),  (8, 'Y', False)),
                         'Y':  ((3, 'Y', False),  (5, 'Y', False))},
                     5: {'X':  ((2, 'X', False),  (7, 'Y', False)),
                         'Y':  ((4, 'Y', False),  (6, 'Y', False))},
                     6: {'X':  ((2, 'Y', False),  (7, 'X', False)),
                         'Y':  ((5, 'Y', False),  (10, 'X', False))},
                     7: {'X':  ((6, 'X', False),  (8, 'X', False)),
                         'Y':  ((5, 'X', False),  (10, 'Y', False))},
                     8: {'X':  ((7, 'X', False),  (9, 'X', False)),
                         'Y':  ((4, 'X', False),  (11, 'Y', False))},
                     9: {'X':  ((8, 'X', False),  None),
                         'Y':  ((3, 'X', False),  (12, 'Y', False))},
                     10: {'X': ((6, 'Y', False),  (11, 'X', False)),
                          'Y': ((7, 'Y', False),  (2, 'X', False))},
                     11: {'X': ((10, 'X', False), (12, 'X', False)),
                          'Y': ((8, 'Y', False),  (1, 'X', False))},
                     12: {'X': ((11, 'X', False), None),
                          'Y': ((9, 'Y', False),  (0, 'X', False))}}}

# create the grid object
grid = xgcm.Grid(ds, coords={'X':{'center':'i'}, 'Y':{'center':'j'}}, face_connections=face_connections)
grid
```

gives

<details>
---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
Cell In[24], line 38
      9 face_connections = {'face':
     10                     {0: {'X':  ((12, 'Y', False), (3, 'X', False)),
     11                          'Y':  (None,             (1, 'Y', False))},
   (...)
     34                      12: {'X': ((11, 'X', False), None),
     35                           'Y': ((9, 'Y', False),  (0, 'X', False))}}}
     37 # create the grid object
---> 38 grid = xgcm.Grid(ds, coords={'X':{'center':'i'}, 'Y':{'center':'j'}}, face_connections=face_connections)
     39 grid

File /srv/conda/envs/notebook/lib/python3.10/site-packages/xgcm/grid.py:1412, in Grid.__init__(self, ds, check_dims, periodic, default_shifts, face_connections, coords, metrics, boundary, fill_value)
   1401     self.axes[axis_name] = Axis(
   1402         ds,
   1403         axis_name,
   (...)
   1408         fill_value=axis_fillvalue,
   1409     )
   1411 if face_connections is not None:
-> 1412     self._assign_face_connections(face_connections)
   1414 self._metrics = {}
   1416 if metrics is not None:

File /srv/conda/envs/notebook/lib/python3.10/site-packages/xgcm/grid.py:1484, in Grid._assign_face_connections(self, fc)
   1481 axis_connections = {}
   1483 facedim = list(fc.keys())[0]
-> 1484 assert facedim in self._ds
   1486 face_links = fc[facedim]
   1487 for fidx, face_axis_links in face_links.items():

AssertionError:
</details>

but if I assign actual coordinates to the face dimension

```python
ds = ds.assign_coords(face=np.arange(13))
```

this works. 

I am pretty sure that we are using an ill-posed check [here](assert facedim in self._ds) and that this should be 

```python
assert facedim in self._ds.dims
```

While fixing this we should probably also refactor this to spit out a proper error and not just use an `assert`.



Pull request:
Allow face connections for non-coordinate face dimension
<!-- Feel free to remove check-list items aren't relevant to your change -->

 - [x] Closes #616
 - [x] Tests added
 - [x] Passes `pre-commit run --all-files`
 - [x] User visible changes (including notable bug fixes) are documented in `whats-new.rst`


Patch:
diff --git a/doc/whats-new.rst b/doc/whats-new.rst
index 0454114..c16b02f 100644
--- a/doc/whats-new.rst
+++ b/doc/whats-new.rst
@@ -39,6 +39,10 @@ Documentation
 
 Bugfixes
 ~~~~~~~~
+- Fix bug that did not allow to create grids with faceconnections if the face dimension was coordinate-less. (:issue:`616`, :pull:`616`).
+  By `Julius Busecke <https://github.com/jbusecke>`_.
+
+
 - 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>`_.
 
diff --git a/xgcm/grid.py b/xgcm/grid.py
index 85ec0f9..b66b403 100644
--- a/xgcm/grid.py
+++ b/xgcm/grid.py
@@ -340,7 +340,10 @@ class Grid:
         axis_connections = {}
 
         facedim = list(fc.keys())[0]
-        assert facedim in self._ds
+        if facedim not in self._ds.dims:
+            raise ValueError(
+                f"Face dimension {facedim} does not exist in the dataset. Found {list(self._ds.dims)} instead"
+            )
 
         face_links = fc[facedim]
         for fidx, face_axis_links in face_links.items():


Test patch:
diff --git a/xgcm/test/test_faceconnections.py b/xgcm/test/test_faceconnections.py
index 22562cc..78e7331 100644
--- a/xgcm/test/test_faceconnections.py
+++ b/xgcm/test/test_faceconnections.py
@@ -131,7 +131,12 @@ def test_connection_errors(ds):
     pass
 
 
-def test_create_connected_grid(ds, ds_face_connections_x_to_x):
+@pytest.mark.parametrize("face_coord_dim", [True, False])
+def test_create_connected_grid(ds, ds_face_connections_x_to_x, face_coord_dim):
+    #
+    if face_coord_dim:
+        ds = ds.drop("face")
+
     # simplest scenario with one face connection
     grid = Grid(ds, face_connections=ds_face_connections_x_to_x)
 
@@ -147,6 +152,15 @@ def test_create_connected_grid(ds, ds_face_connections_x_to_x):
     assert xaxis._connections[1][0][1] is xaxis
 
 
+def test_create_connected_grid_error_wrong_facedim(ds, ds_face_connections_x_to_x):
+    # rename face dimension to trigger error
+    ds = ds.rename({"face": "something_else"})
+    with pytest.raises(
+        ValueError, match="Face dimension face does not exist in the dataset."
+    ):
+        Grid(ds, face_connections=ds_face_connections_x_to_x)
+
+
 def test_diff_interp_connected_grid_x_to_x(ds, ds_face_connections_x_to_x):
     # simplest scenario with one face connection
     grid = Grid(ds, face_connections=ds_face_connections_x_to_x, periodic=False)