Merge pull request #124 from noaa-ocs-modeling/bugfix/wintemp

Fixing Permission Issues on Windows Related to Tempfiles

.github/workflows/functional_test.yml

@@ -27,7 +27,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ ubuntu-latest ]
+        os: [ ubuntu-latest, windows-latest ]
         python-version: [ '3.9', '3.10', '3.11']
 
     steps:
@@ -43,22 +43,22 @@ jobs:
 
     - name: Install other dependencies
       shell: bash -l {0}
-      run: |
-        pip install packaging # jigsaw dependency in its setup.py
-        python ./setup.py install_jigsaw
-        pip install .
+      run: pip install .
 
     - name: Prepare example DEM
+      if: runner.os != 'Windows'
       shell: bash -l {0}
       run: |
         wget https://coast.noaa.gov/htdata/raster2/elevation/NCEI_ninth_Topobathy_2014_8483/northeast_sandy/ncei19_n40x75_w073x75_2015v1.tif -O /tmp/fullsize_dem.tif
         gdalwarp -tr 0.0005 0.0005 -r average -overwrite /tmp/fullsize_dem.tif /tmp/test_dem.tif
 
     - name: Export Conda environment
-      shell: bash -l {0}
+      if: runner.os != 'Windows'
+      shell: bash -el {0}
       run: conda list > environment_py${{ matrix.python-version }}.yml
 
     - name: 'Upload conda environment'
+      if: runner.os != 'Windows'
       uses: actions/upload-artifact@v2
       with:
         name: conda-environment-${{ matrix.python-version }}
@@ -67,45 +67,52 @@ jobs:
 
 
     - name: 'Upload test dem'
+      if: runner.os != 'Windows'
       uses: actions/upload-artifact@v2
       with:
         name: test-dem-${{ matrix.python-version }}
         path: /tmp/test_dem.tif
         retention-days: 7
 
     - name: Run geom build test
+      if: runner.os != 'Windows'
       shell: bash -l {0}
       run: source tests/cli/build_geom.sh
 
     - name: 'Upload Geom build results'
+      if: runner.os != 'Windows'
       uses: actions/upload-artifact@v2
       with:
         name: geom-build-results-${{ matrix.python-version }}
         path: test_shape
         retention-days: 7
 
     - name: Run hfun build test
+      if: runner.os != 'Windows'
       shell: bash -l {0}
       run: source tests/cli/build_hfun.sh
 
     - name: 'Upload Hfun build results'
+      if: runner.os != 'Windows'
       uses: actions/upload-artifact@v2
       with:
         name: hfun-build-results-${{ matrix.python-version }}
         path: test.2dm
         retention-days: 7
 
     - name: Run remesh test
+      if: runner.os != 'Windows'
       shell: bash -l {0}
       run: source tests/cli/remesh_by_dem.sh
 
     - name: 'Upload remesh results'
+      if: runner.os != 'Windows'
       uses: actions/upload-artifact@v2
       with:
         name: remesh-bydem-results-${{ matrix.python-version }}
         path: remeshed.2dm
         retention-days: 7
 
     - name: Run Python API tests
-      shell: bash -l {0}
+      shell: bash -el {0}
       run: python -m unittest discover tests/api -p "*.py"

.github/workflows/functional_test_2.yml

@@ -28,7 +28,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ ubuntu-latest ]
-        python-version: [ '3.9', '3.10', '3.x' ]
+        python-version: [ '3.9', '3.10', '3.11' ]
 
     steps:
     - name: Checkout 

environment.yml

@@ -23,3 +23,4 @@ dependencies:
   - pytz
   - colored-traceback
   - typing-extensions
+  - jigsawpy

ocsmesh/hfun/collector.py

@@ -832,6 +832,10 @@ def msh_t(self) -> jigsaw_msh_t:
                 rast = self._create_big_raster(temp_dir)
                 hfun = self._apply_features_fast(rast)
                 composite_hfun = self._get_hfun_composite_fast(hfun)
+                # So that the tempfiles are deleted and the dir can be
+                # safely removed
+                del rast
+                del hfun
 
         else:
             raise ValueError(f"Invalid method specified: {self._method}")

ocsmesh/hfun/raster.py

@@ -4,8 +4,10 @@
 import functools
 import gc
 import logging
+import os
 from multiprocessing import cpu_count, Pool
 import operator
+import pathlib
 import tempfile
 from time import time
 from typing import Union, List, Callable, Optional, Iterable, Tuple
@@ -234,6 +236,11 @@ def __init__(self,
         self._constraints = []
 
 
+    def __del__(self):
+        for _, memfile_path in self._xy_cache.items():
+            pathlib.Path(memfile_path).unlink()
+
+
     def msh_t(
             self,
             window: Optional[rasterio.windows.Window] = None,
@@ -1296,15 +1303,17 @@ def get_xy_memcache(
             transformer = Transformer.from_crs(
                 self.src.crs, dst_crs, always_xy=True)
             # pylint: disable=R1732
-            tmpfile = tempfile.NamedTemporaryFile()
+#            tmpfile = tempfile.NamedTemporaryFile()
+            tmpfd, tmppath = tempfile.mkstemp()
             xy = self.get_xy(window)
-            fp = np.memmap(tmpfile, dtype='float32', mode='w+', shape=xy.shape)
+            fp = np.memmap(tmppath, dtype='float32', mode='w+', shape=xy.shape)
+            os.close(tmpfd)
             fp[:] = np.vstack(
                 transformer.transform(xy[:, 0], xy[:, 1])).T
             _logger.info('Saving values to memcache...')
             fp.flush()
             _logger.info('Done!')
-            self._xy_cache[f'{window}{dst_crs}'] = tmpfile
+            self._xy_cache[f'{window}{dst_crs}'] = tmppath
             return fp[:]
 
         _logger.info('Loading values from memcache...')

ocsmesh/ops/combine_geom.py

@@ -163,16 +163,17 @@ def run(self):
 
         poly_files_coll = []
         _logger.info(f"Number of processes: {nprocs}")
-        with tempfile.TemporaryDirectory(dir=out_dir) as temp_dir, \
-                tempfile.NamedTemporaryFile() as base_file:
+        with tempfile.TemporaryDirectory(dir=out_dir) as temp_dir:
 
+            tmpfd, tmppath = tempfile.mkstemp()
             if base_mult_poly:
-                base_mesh_path = base_file.name
+                base_mesh_path = tmppath
                 self._multipolygon_to_disk(
                     base_mesh_path, base_mult_poly, fix=False)
             else:
                 base_mesh_path = None
             base_mult_poly = None
+            os.close(tmpfd)
 
 
             _logger.info("Processing DEM priorities ...")
@@ -235,6 +236,7 @@ def run(self):
                     ],
                     ignore_index=True
                 )
+            pathlib.Path(tmppath).unlink()
 
 
             # The assumption is this returns polygon or multipolygon

ocsmesh/raster.py

@@ -13,6 +13,7 @@
 import pathlib
 import tempfile
 import warnings
+import platform
 from time import time
 from contextlib import contextmanager, ExitStack
 from typing import (
@@ -144,15 +145,23 @@ class TemporaryFile:
     cleanup capabities on object destruction.
     """
 
-    def __set__(self, obj, val: tempfile.NamedTemporaryFile):
+    def __set__(self, obj, val: Optional[os.PathLike]):
+        tmpfile = obj.__dict__.get('tmpfile')
+        if tmpfile is not None:
+            obj._src = None
+            pathlib.Path(tmpfile).unlink()
+
         obj.__dict__['tmpfile'] = val
-        obj._src = rasterio.open(val.name)
+        if val is None:
+            obj._src = None
+        else:
+            obj._src = rasterio.open(val)
 
     def __get__(self, obj, objtype=None) -> pathlib.Path:
         tmpfile = obj.__dict__.get('tmpfile')
         if tmpfile is None:
             return obj.path
-        return pathlib.Path(tmpfile.name)
+        return pathlib.Path(tmpfile)
 
 
 class SourceRaster:
@@ -165,7 +174,10 @@ class SourceRaster:
     opening it everytime need arises.
     """
 
-    def __set__(self, obj, val: rasterio.DatasetReader):
+    def __set__(self, obj, val: Optional[rasterio.DatasetReader]):
+        source = obj.__dict__.get('source')
+        if source is not None:
+            source.close()
         obj.__dict__['source'] = val
 
     def __get__(self, obj, objtype=None) -> rasterio.DatasetReader:
@@ -345,6 +357,9 @@ def __init__(
         self._path = path
         self._crs = crs
 
+    def __del__(self):
+        self._tmpfile = None
+
     def __iter__(self, chunk_size: int = None, overlap: int = None):
         for window in self.iter_windows(chunk_size, overlap):
             yield window, self.get_window_bounds(window)
@@ -380,16 +395,18 @@ def modifying_raster(
         """
 
         no_except = False
+        tmpfd = None
         try:
             # pylint: disable=R1732
-            tmpfile = tempfile.NamedTemporaryFile(prefix=tmpdir)
+#            tmpfile = tempfile.NamedTemporaryFile(prefix=tmpdir, mode='w')
+            tmpfd, tmppath = tempfile.mkstemp(prefix=tmpdir)
 
             new_meta = kwargs
             # Flag to workaround cases where "src" is NOT set yet
             if use_src_meta:
                 new_meta = self.src.meta.copy()
                 new_meta.update(**kwargs)
-            with rasterio.open(tmpfile.name, 'w', **new_meta) as dst:
+            with rasterio.open(tmppath, 'w', **new_meta) as dst:
                 if use_src_meta:
                     for i, desc in enumerate(self.src.descriptions):
                         dst.set_band_description(i+1, desc)
@@ -399,9 +416,13 @@ def modifying_raster(
 
         finally:
             if no_except:
-                # So that tmpfile is NOT destroyed when it locally
-                # goes out of scope
-                self._tmpfile = tmpfile
+                self._tmpfile = tmppath
+
+            # We don't need to keep the descriptor open, we kept it
+            # open # so that there's no race condition on the temp
+            # file up to now
+            if tmpfd is not None:
+                os.close(tmpfd)
 
 
 
@@ -944,6 +965,8 @@ def average_filter(
         # in other parts of the code. Thorough testing is needed for
         # modifying the raster (e.g. hfun add_contour is affected)
 
+        if platform.system() == 'Windows':
+            raise NotImplementedError('Not supported on Windows!')
         bands = apply_on_bands
         if bands is None:
             bands = range(1, self.src.count + 1)
@@ -1002,6 +1025,9 @@ def generic_filter(self, function, **kwargs: Any) -> None:
         None
         """
 
+        if platform.system() == 'Windows':
+            raise NotImplementedError('Not supported on Windows!')
+
         # TODO: Don't overwrite; add additoinal bands for filtered values
 
         # NOTE: Adding new bands in this function can result in issues

tests/api/__init__.py

@@ -1,7 +1,23 @@
 import os
-os.system("""
-    ls /tmp/test_dem.tif > /dev/null
-    if [ $? -eq 0 ]; then exit; fi
-    wget https://coast.noaa.gov/htdata/raster2/elevation/NCEI_ninth_Topobathy_2014_8483/northeast_sandy/ncei19_n40x75_w073x75_2015v1.tif -O /tmp/fullsize_dem.tif
-    gdalwarp -tr 0.0005 0.0005 -r average -overwrite /tmp/fullsize_dem.tif /tmp/test_dem.tif
-""")
+import tempfile
+import urllib.request
+from pathlib import Path
+
+from rasterio.enums import Resampling
+
+from ocsmesh.raster import Raster
+
+
+# Find a better way!
+tif_url = (
+    'https://coast.noaa.gov/htdata/raster2/elevation/NCEI_ninth_Topobathy_2014_8483/northeast_sandy/ncei19_n40x75_w073x75_2015v1.tif'
+)
+TEST_FILE = os.path.join(tempfile.gettempdir(), 'test_dem.tif')
+if not Path(TEST_FILE).exists():
+    tmpfd, tmppath = tempfile.mkstemp()
+    urllib.request.urlretrieve(tif_url, filename=tmppath)
+    os.close(tmpfd)
+    r = Raster(tmppath)
+    r.resampling_method = Resampling.average
+    r.resample(scaling_factor=0.01)
+    r.save(TEST_FILE)

tests/api/hfun.py

@@ -13,6 +13,7 @@
 
 import ocsmesh
 
+from tests.api import TEST_FILE
 from tests.api.common import (
     topo_2rast_1mesh,
 )
@@ -492,7 +493,7 @@ def test_hfun_fast_extent(self):
 class SizeFromMesh(unittest.TestCase):
 
     def setUp(self):
-        rast = ocsmesh.raster.Raster('/tmp/test_dem.tif')
+        rast = ocsmesh.raster.Raster(TEST_FILE)
 
         hfun_orig = ocsmesh.hfun.hfun.Hfun(rast, hmin=100, hmax=1500)
         hfun_orig.add_contour(level=0, expansion_rate=0.001, target_size=100)
@@ -512,8 +513,8 @@ def test_calculated_size(self):
         hfun_val_diff = self.hfun_orig_val - hfun_calc_val
 
         # TODO: Come up with a more robust criteria!
-        threshold = 0.21
-        err_value = np.max(np.abs(hfun_val_diff))/np.max(self.hfun_orig_val)
+        threshold = 0.05
+        err_value = np.mean(np.abs(hfun_val_diff))/np.mean(self.hfun_orig_val)
         self.assertTrue(err_value < threshold)
 
 
@@ -530,7 +531,7 @@ def test_hfun_raster_cfl_constraint_support(self):
         courant_hi = 0.8
         courant_lo = 0.2
 
-        rast = ocsmesh.raster.Raster('/tmp/test_dem.tif')
+        rast = ocsmesh.raster.Raster(TEST_FILE)
 
         hfun_raster = ocsmesh.hfun.hfun.Hfun(rast, hmin=100, hmax=5000)
         hfun_raster.add_courant_num_constraint(
@@ -586,7 +587,7 @@ def test_hfun_raster_cfl_constraint_support(self):
         )
 
     def test_hfun_raster_cfl_constraint_io(self):
-        rast = ocsmesh.raster.Raster('/tmp/test_dem.tif')
+        rast = ocsmesh.raster.Raster(TEST_FILE)
         hfun_raster = ocsmesh.hfun.hfun.Hfun(rast, hmin=100, hmax=5000)
         self.assertRaises(
             ValueError,
@@ -614,8 +615,8 @@ def test_hfun_coll_cfl_constraint(self):
             # TODO: Add subTest
 
             # Creating adjacent rasters from the test raster
-            rast1 = ocsmesh.raster.Raster('/tmp/test_dem.tif')
-            rast2 = ocsmesh.raster.Raster('/tmp/test_dem.tif')
+            rast1 = ocsmesh.raster.Raster(TEST_FILE)
+            rast2 = ocsmesh.raster.Raster(TEST_FILE)
             bounds = rast1.bbox.bounds
             bbox1 = geometry.box(
                 bounds[0], bounds[1], (bounds[0] + bounds[2]) / 2, bounds[3]
@@ -906,7 +907,6 @@ def setUp(self):
             )
             mesh = ocsmesh.Mesh(msh_t)
             mesh.write(str(self.mesh1), format='grd', overwrite=False)
-            mesh.write('/tmp/ocsmesh/mytest2.2dm', format='2dm', overwrite=True)
 
 
     def tearDown(self):