Merge pull request #314 from Deltares/dataset-lock

Fix #309. Use xarray's set_close consistently. Silence some additional xarray FutureWarnings

docs/changelog.rst

@@ -38,6 +38,12 @@ Fixed
   :class:`xugrid.OverlapRegridder`, :class:`xugrid.RelativeOverlapRegridder`,
   and :class:`xugrid.BarycentricInterpolator` now raise a TypeError if an
   inappropriate type is provided.
+- Fixed file handling in :meth:`xugrid.UgridDataArray.close` and
+  :meth:`xugrid.UgridDataset.close`. Previously, files opened with
+  :func:`xugrid.open_dataarray` or :func:`xugrid.open_dataset` could not be
+  properly closed, and new UgridDataset or UgridDataArray objects were not
+  correctly associated with their source files. Now, calling the close methods
+  will properly close the associated files.
 
 [0.12.1] 2024-09-09
 -------------------

tests/test_ugrid_dataset.py

@@ -1,3 +1,4 @@
+import os
 import warnings
 
 import dask
@@ -44,6 +45,7 @@ def DARRAY():
     return xr.DataArray(
         data=np.ones(GRID().n_face),
         dims=[GRID().face_dimension],
+        name="a",
     )
 
 
@@ -140,6 +142,13 @@ def test_dunder_forward(self):
         assert isinstance(int(self.uda[0]), int)
         assert isinstance(float(self.uda[0]), float)
 
+    def test_close(self, tmp_path):
+        path = tmp_path / "dataarray-closetest.nc"
+        self.uda.ugrid.to_netcdf(path)
+        back = xugrid.open_dataarray(path)
+        back.close()
+        os.remove(path)
+
     def test_repr(self):
         assert self.uda.__repr__() == self.uda.obj.__repr__()
 
@@ -311,8 +320,10 @@ def test_is_geographic(self):
         assert result.grid.is_geographic is False
 
     def test_to_geodataframe(self):
+        uda1 = self.uda.copy()
+        uda1.ugrid.obj.name = None
         with pytest.raises(ValueError, match="unable to convert unnamed"):
-            self.uda.ugrid.to_geodataframe()
+            uda1.ugrid.to_geodataframe()
         uda2 = self.uda.copy()
         uda2.ugrid.obj.name = "test"
         uda2.ugrid.set_crs(epsg=28992)
@@ -487,6 +498,13 @@ def test_init_from_dataset_only(self):
     def test_repr(self):
         assert self.uds.__repr__() == self.uds.obj.__repr__()
 
+    def test_close(self, tmp_path):
+        path = tmp_path / "dataset-closetest.nc"
+        self.uds.ugrid.to_netcdf(path)
+        back = xugrid.open_dataset(path)
+        back.close()
+        os.remove(path)
+
     def test_getitem(self):
         assert "a" in self.uds
         assert "b" in self.uds
@@ -627,7 +645,7 @@ def test_intersect_line(self):
         actual = self.uds.ugrid.intersect_line(start=p0, end=p1)
         sqrt2 = np.sqrt(2.0)
         assert isinstance(actual, xr.Dataset)
-        assert actual.dims == {"mesh2d_nFaces": 2}
+        assert actual.sizes == {"mesh2d_nFaces": 2}
         assert np.allclose(actual["mesh2d_x"], [0.5, 1.25])
         assert np.allclose(actual["mesh2d_y"], [0.5, 1.25])
         assert np.allclose(actual["mesh2d_s"], [0.5 * sqrt2, 1.25 * sqrt2])
@@ -644,7 +662,7 @@ def test_intersect_linestring(self):
         )
         actual = self.uds.ugrid.intersect_linestring(linestring)
         assert isinstance(actual, xr.Dataset)
-        assert actual.dims == {"mesh2d_nFaces": 4}
+        assert actual.sizes == {"mesh2d_nFaces": 4}
         assert np.allclose(actual["mesh2d_x"], [0.75, 1.25, 1.5, 1.5])
         assert np.allclose(actual["mesh2d_y"], [0.5, 0.5, 0.75, 1.25])
         assert np.allclose(actual["mesh2d_s"], [0.25, 0.75, 1.25, 1.75])

xugrid/core/wrap.py

@@ -205,6 +205,7 @@ def __init__(self, obj: xr.DataArray, grid: UgridType):
 
         self._grid = grid
         self._obj = assign_ugrid_coords(obj, [grid])
+        self._obj.set_close(obj._close)
 
     def __getattr__(self, attr):
         result = getattr(self.obj, attr)
@@ -311,7 +312,7 @@ def __init__(
             raise ValueError("At least either obj or grids is required")
 
         if obj is None:
-            ds = xr.Dataset()
+            original = ds = xr.Dataset()
         else:
             if not isinstance(obj, xr.Dataset):
                 raise TypeError(
@@ -324,6 +325,7 @@ def __init__(
                 for name in v.values()
             ]
             ds = obj.drop_vars(obj.ugrid_roles.topology + connectivity_vars)
+            original = obj
 
         if grids is None:
             topologies = obj.ugrid_roles.topology
@@ -344,6 +346,10 @@ def __init__(
 
         self._grids = grids
         self._obj = assign_ugrid_coords(ds, grids)
+        # We've created a new object; the file handle will be associated with the original.
+        # set_close makes sure that when close is called on the UgridDataset, that the
+        # file will actually be closed (via the original).
+        self._obj.set_close(original._close)
 
     @property
     def obj(self):