Repository: UXARRAY/uxarray

Issue:
Rename `face_dimension` connectivity to `nNodes_per_face`
Current implementation of `face_dimension` does not align with the UGRID conventions. `nNodes_per_face` is a more appropriate name for the current implementation of `face_dimension`

Discussion:

Patch:
diff --git a/docs/internal_api/index.rst b/docs/internal_api/index.rst
index b6f2a237..68251aef 100644
--- a/docs/internal_api/index.rst
+++ b/docs/internal_api/index.rst
@@ -23,7 +23,7 @@ Grid Methods
    grid.Grid._build_face_edges_connectivity
    grid.Grid._populate_cartesian_xyz_coord
    grid.Grid._populate_lonlat_coord
-   grid.Grid._build_face_dimension
+   grid.Grid._build_nNodes_per_face
 
 
 Grid Helper Modules
diff --git a/uxarray/grid.py b/uxarray/grid.py
index f23b5179..21f9b548 100644
--- a/uxarray/grid.py
+++ b/uxarray/grid.py
@@ -103,7 +103,7 @@ class Grid:
         self._build_edge_node_connectivity()
 
         # build face dimension, possibly safeguard for large datasets
-        self._build_face_dimension()
+        self._build_nNodes_per_face()
 
     def __init_ds_var_names__(self):
         """Populates a dictionary for storing uxarray's internal representation
@@ -371,7 +371,7 @@ class Grid:
             coords_type = "cartesian"
 
         face_nodes = self.Mesh2_face_nodes.data
-        face_dimension = self.Mesh2_face_dimension.data
+        nNodes_per_face = self.nNodes_per_face.data
         dim = self.Mesh2.attrs['topology_dimension']
 
         # initialize z
@@ -393,7 +393,7 @@ class Grid:
 
         # call function to get area of all the faces as a np array
         self._face_areas = get_all_face_area_from_coords(
-            x, y, z, face_nodes, face_dimension, dim, quadrature_rule, order,
+            x, y, z, face_nodes, nNodes_per_face, dim, quadrature_rule, order,
             coords_type)
 
         return self._face_areas
@@ -700,11 +700,11 @@ class Grid:
                 "units": "degrees_north",
             })
 
-    def _build_face_dimension(self):
-        """Constructs ``Mesh2_face_dimension``, which calculates the dimension
-        of each face in ``Mesh2_face_nodes``"""
+    def _build_nNodes_per_face(self):
+        """Constructs ``nNodes_per_face``, which contains the number of non-
+        fill-value nodes for each face in ``Mesh2_face_nodes``"""
 
-        # Triangular Mesh
+        # Triangular Mesh (No Fill Values)
         if not hasattr(self, "nMaxMesh2_face_nodes"):
             nMaxMesh2_face_nodes = self.Mesh2_face_nodes.shape[1]
             setattr(self, "nMaxMesh2_face_nodes", nMaxMesh2_face_nodes)
@@ -715,13 +715,13 @@ class Grid:
 
         closed[:, :-1] = self.Mesh2_face_nodes.copy()
 
-        face_dimension = np.argmax(closed == INT_FILL_VALUE, axis=1)
+        nNodes_per_face = np.argmax(closed == INT_FILL_VALUE, axis=1)
 
         # add to internal dataset
-        self.ds["Mesh2_face_dimension"] = xr.DataArray(
-            data=face_dimension,
+        self.ds["nNodes_per_face"] = xr.DataArray(
+            data=nNodes_per_face,
             dims=["nMesh2_face"],
             attrs={"long_name": "number of non-fill value nodes for each face"})
 
         # standardized attribute
-        setattr(self, "Mesh2_face_dimension", self.ds["Mesh2_face_dimension"])
+        setattr(self, "nNodes_per_face", self.ds["nNodes_per_face"])


Test patch:
diff --git a/test/test_grid.py b/test/test_grid.py
index 92d32785..d4d0d512 100644
--- a/test/test_grid.py
+++ b/test/test_grid.py
@@ -286,20 +286,6 @@ class TestGrid(TestCase):
         xr_grid_u30 = xr.open_dataset(ug_30)
         ux_grid_u30 = ux.Grid(xr_grid_u30)  # tests from ugrid
 
-    def test_build_face_dimension(self):
-        """Tests the construction of the ``Mesh2_face_dimension`` variable."""
-        grids = [self.tgrid1, self.tgrid2, self.tgrid3]
-
-        for grid in grids:
-            # highest possible dimension dimension for a face
-            max_dimension = grid.nMaxMesh2_face_nodes
-
-            # face must be at least a triangle
-            min_dimension = 3
-
-            assert grid.Mesh2_face_dimension.min() >= min_dimension
-            assert grid.Mesh2_face_dimension.max() <= max_dimension
-
 
 class TestIntegrate(TestCase):
     mesh_file30 = current_path / "meshfiles" / "ugrid" / "outCSne30" / "outCSne30.ug"
@@ -497,6 +483,32 @@ class TestConnectivity(TestCase):
     ugrid_filepath_02 = current_path / "meshfiles" / "ugrid" / "outRLL1deg" / "outRLL1deg.ug"
     ugrid_filepath_03 = current_path / "meshfiles" / "ugrid" / "ov_RLL10deg_CSne4" / "ov_RLL10deg_CSne4.ug"
 
+    xrds_maps = xr.open_dataset(mpas_filepath)
+    xrds_exodus = xr.open_dataset(exodus_filepath)
+    xrds_ugrid = xr.open_dataset(ugrid_filepath_01)
+
+    grid_mpas = ux.Grid(xrds_maps)
+    grid_exodus = ux.Grid(xrds_exodus)
+    grid_ugrid = ux.Grid(xrds_ugrid)
+
+    # used from constructing vertices
+    f0_deg = [[120, -20], [130, -10], [120, 0], [105, 0], [95, -10], [105, -20]]
+    f1_deg = [[120, 0], [120, 10], [115, 0],
+              [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE],
+              [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE],
+              [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE]]
+    f2_deg = [[115, 0], [120, 10], [100, 10], [105, 0],
+              [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE],
+              [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE]]
+    f3_deg = [[95, -10], [105, 0], [95, 30], [80, 30], [70, 0], [75, -10]]
+    f4_deg = [[65, -20], [75, -10], [70, 0], [55, 0], [45, -10], [55, -20]]
+    f5_deg = [[70, 0], [80, 30], [70, 30], [60, 0],
+              [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE],
+              [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE]]
+    f6_deg = [[60, 0], [70, 30], [40, 30], [45, 0],
+              [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE],
+              [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE]]
+
     # Helper function
     def _revert_edges_conn_to_face_nodes_conn(
             self, edge_nodes_connectivity: np.ndarray,
@@ -593,6 +605,34 @@ class TestConnectivity(TestCase):
 
         return np.array(res_face_nodes_connectivity)
 
+    def test_build_nNodes_per_face(self):
+        """Tests the construction of the ``nNodes_per_face`` variable."""
+
+        # test on grid constructed from sample datasets
+        grids = [self.grid_mpas, self.grid_exodus, self.grid_ugrid]
+
+        for grid in grids:
+            # highest possible dimension dimension for a face
+            max_dimension = grid.nMaxMesh2_face_nodes
+
+            # face must be at least a triangle
+            min_dimension = 3
+
+            assert grid.nNodes_per_face.min() >= min_dimension
+            assert grid.nNodes_per_face.max() <= max_dimension
+
+        # test on grid constructed from vertices
+        verts = [
+            self.f0_deg, self.f1_deg, self.f2_deg, self.f3_deg, self.f4_deg,
+            self.f5_deg, self.f6_deg
+        ]
+        grid_from_verts = ux.Grid(verts)
+
+        # number of non-fill-value nodes per face
+        expected_nodes_per_face = np.array([6, 3, 4, 6, 6, 4, 4], dtype=int)
+        nt.assert_equal(grid_from_verts.nNodes_per_face.values,
+                        expected_nodes_per_face)
+
     def test_edge_nodes_euler(self):
         """Verifies that (``nMesh2_edge``) follows euler's formula."""
         grid_paths = [
@@ -703,25 +743,10 @@ class TestConnectivity(TestCase):
         self.assertEqual(mesh2_edge_nodes.sizes["nMesh2_edge"], num_edges)
 
     def test_build_face_edges_connectivity_fillvalues(self):
-        f0_deg = [[120, -20], [130, -10], [120, 0], [105, 0], [95, -10],
-                  [105, -20]]
-        f1_deg = [[120, 0], [120, 10], [115, 0],
-                  [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE],
-                  [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE],
-                  [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE]]
-        f2_deg = [[115, 0], [120, 10], [100, 10], [105, 0],
-                  [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE],
-                  [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE]]
-        f3_deg = [[95, -10], [105, 0], [95, 30], [80, 30], [70, 0], [75, -10]]
-        f4_deg = [[65, -20], [75, -10], [70, 0], [55, 0], [45, -10], [55, -20]]
-        f5_deg = [[70, 0], [80, 30], [70, 30], [60, 0],
-                  [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE],
-                  [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE]]
-        f6_deg = [[60, 0], [70, 30], [40, 30], [45, 0],
-                  [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE],
-                  [ux.INT_FILL_VALUE, ux.INT_FILL_VALUE]]
-
-        verts = [f0_deg, f1_deg, f2_deg, f3_deg, f4_deg, f5_deg, f6_deg]
+        verts = [
+            self.f0_deg, self.f1_deg, self.f2_deg, self.f3_deg, self.f4_deg,
+            self.f5_deg, self.f6_deg
+        ]
         uds = ux.Grid(verts)
         uds._build_face_edges_connectivity()
         n_face = len(uds.ds["Mesh2_face_edges"].values)