From a94bd0d51245b2a997039061370dc2fc28aab437 Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Wed, 31 Jan 2024 18:16:51 +0100 Subject: [PATCH 1/7] Check segmentation fault cause --- dev-environment.yml | 1 + environment.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/dev-environment.yml b/dev-environment.yml index 7d45fa5d..014174da 100644 --- a/dev-environment.yml +++ b/dev-environment.yml @@ -12,6 +12,7 @@ dependencies: - tqdm - xarray - rioxarray + - pandas # Development-specific, to mirror manually in setup.cfg [options.extras_require]. - pip diff --git a/environment.yml b/environment.yml index 1940594e..ffd01001 100644 --- a/environment.yml +++ b/environment.yml @@ -12,3 +12,4 @@ dependencies: - tqdm - xarray - rioxarray + - pandas From 79a4f233b8ab6e90ade7861a6c2619dbc4e34e57 Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Wed, 31 Jan 2024 18:17:34 +0100 Subject: [PATCH 2/7] Even better like this --- dev-environment.yml | 1 - environment.yml | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/dev-environment.yml b/dev-environment.yml index 014174da..7d45fa5d 100644 --- a/dev-environment.yml +++ b/dev-environment.yml @@ -12,7 +12,6 @@ dependencies: - tqdm - xarray - rioxarray - - pandas # Development-specific, to mirror manually in setup.cfg [options.extras_require]. - pip diff --git a/environment.yml b/environment.yml index ffd01001..e0234412 100644 --- a/environment.yml +++ b/environment.yml @@ -1,7 +1,7 @@ name: geoutils channels: - conda-forge -dependencies: +dependencies: # LOL - python>=3.9,<3.12 - geopandas>=0.12.0 - matplotlib @@ -12,4 +12,3 @@ dependencies: - tqdm - xarray - rioxarray - - pandas From 8c94b0114f67af9054c7f11131d4319671ba49ad Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Wed, 31 Jan 2024 18:18:43 +0100 Subject: [PATCH 3/7] Bump cache --- .github/workflows/python-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 4872797e..9b3a72d7 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -48,7 +48,7 @@ jobs: path: ${{ env.CONDA }}/envs key: conda-${{ matrix.os }}-${{ matrix.python-version }}-${{ env.cache_date }}-${{ hashFiles('dev-environment.yml') }}-${{ env.CACHE_NUMBER }} env: - CACHE_NUMBER: 0 # Increase this value to reset cache if environment.yml has not changed + CACHE_NUMBER: 1 # Increase this value to reset cache if environment.yml has not changed id: cache # The trick below is necessary because the generic environment file does not specify a Python version, and ONLY From 9289da62a2f9b84fb8856f81ed503ba1db7d8e67 Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Wed, 31 Jan 2024 18:33:40 +0100 Subject: [PATCH 4/7] With the right name --- .github/workflows/python-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 9b3a72d7..62b8a00a 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -60,7 +60,7 @@ jobs: run: | mamba install pyyaml python=${{ matrix.python-version }} python .github/scripts/generate_yml_env_fixed_py.py --pyv ${{ matrix.python-version }} --add "graphviz" "environment.yml" - mamba env update -n xdem-dev -f environment-ci-py${{ matrix.python-version }}.yml + mamba env update -n geoutils-dev -f environment-ci-py${{ matrix.python-version }}.yml - name: Install project run: pip install -e . --no-dependencies From 11c000651b03e0f0c843847235fbcacb97b72d17 Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Thu, 1 Feb 2024 14:08:39 +0100 Subject: [PATCH 5/7] Fix raster_equality and mask saving --- geoutils/raster/raster.py | 22 +++++++++++++++++----- tests/test_raster.py | 28 +++++++++++++++++++--------- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/geoutils/raster/raster.py b/geoutils/raster/raster.py index 26816b95..f02434db 100644 --- a/geoutils/raster/raster.py +++ b/geoutils/raster/raster.py @@ -986,12 +986,23 @@ def raster_equal(self, other: object) -> bool: - The raster's transform, crs and nodata values. """ + # If the mask is just "False", it is equivalent to being equal to an array of False + if isinstance(self.data.mask, np.bool_): + self_mask = np.zeros(np.shape(self.data), dtype=bool) + else: + self_mask = self.data.mask + + if isinstance(other.data.mask, np.bool_): + other_mask = np.zeros(np.shape(other.data), dtype=bool) + else: + other_mask = other.data.mask + if not isinstance(other, Raster): # TODO: Possibly add equals to SatelliteImage? raise NotImplementedError("Equality with other object than Raster not supported by raster_equal.") return all( [ np.array_equal(self.data.data, other.data.data, equal_nan=True), - np.array_equal(self.data.mask, other.data.mask), + np.array_equal(self_mask, other_mask), self.data.fill_value == other.data.fill_value, self.data.dtype == other.data.dtype, self.transform == other.transform, @@ -3585,14 +3596,15 @@ def __init__( ) self._data = self.data[0, :, :] - # Convert masked array to boolean - self._data = self.data.astype(bool) # type: ignore + # Force dtypes + self._dtypes = (bool,) # Fix nodata to None self._nodata = None - # Define in dtypes - self._dtypes = (bool,) + # Convert masked array to boolean + self._data = self.data.astype(bool) # type: ignore + def __repr__(self) -> str: """Convert mask to string representation.""" diff --git a/tests/test_raster.py b/tests/test_raster.py index d8c5b8dd..2ddf5a39 100644 --- a/tests/test_raster.py +++ b/tests/test_raster.py @@ -2960,12 +2960,15 @@ def test_reproject(self, mask: gu.Mask) -> None: # Test 1: with a classic resampling (bilinear) # Reproject mask - resample to 100 x 100 grid + mask_orig = mask.copy() mask_reproj = mask.reproject(grid_size=(100, 100), force_source_nodata=2) # Check instance is respected assert isinstance(mask_reproj, gu.Mask) # Check the dtype of the original mask was properly reconverted assert mask.data.dtype == bool + # Check the original mask was not modified during reprojection + assert mask_orig.raster_equal(mask) # Check inplace behaviour works mask_tmp = mask.copy() @@ -2991,6 +2994,8 @@ def test_reproject(self, mask: gu.Mask) -> None: @pytest.mark.parametrize("mask", [mask_landsat_b4, mask_aster_dem, mask_everest]) # type: ignore def test_crop(self, mask: gu.Mask) -> None: # Test with same bounds -> should be the same # + + mask_orig = mask.copy() crop_geom = mask.bounds mask_cropped = mask.crop(crop_geom) assert mask_cropped.raster_equal(mask) @@ -2999,6 +3004,8 @@ def test_crop(self, mask: gu.Mask) -> None: assert isinstance(mask_cropped, gu.Mask) # Check the dtype of the original mask was properly reconverted assert mask.data.dtype == bool + # Check the original mask was not modified during cropping + assert mask_orig.raster_equal(mask) # Check inplace behaviour works mask_tmp = mask.copy() @@ -3054,10 +3061,14 @@ def test_crop(self, mask: gu.Mask) -> None: @pytest.mark.parametrize("mask", [mask_landsat_b4, mask_aster_dem, mask_everest]) # type: ignore def test_polygonize(self, mask: gu.Mask) -> None: + + mask_orig = mask.copy() # Run default vect = mask.polygonize() # Check the dtype of the original mask was properly reconverted assert mask.data.dtype == bool + # Check the original mask was not modified during polygonizing + assert mask_orig.raster_equal(mask) # Check the output is cast into a vector assert isinstance(vect, gu.Vector) @@ -3072,10 +3083,14 @@ def test_polygonize(self, mask: gu.Mask) -> None: @pytest.mark.parametrize("mask", [mask_landsat_b4, mask_aster_dem, mask_everest]) # type: ignore def test_proximity(self, mask: gu.Mask) -> None: + + mask_orig = mask.copy() # Run default rast = mask.proximity() # Check the dtype of the original mask was properly reconverted assert mask.data.dtype == bool + # Check the original mask was not modified during reprojection + assert mask_orig.raster_equal(mask) # Check that output is cast back into a raster assert isinstance(rast, gu.Raster) @@ -3094,17 +3109,12 @@ def test_save(self, mask: gu.Mask) -> None: mask.save(temp_file) saved = gu.Mask(temp_file) - # TODO: Generalize raster_equal for masks? + # A raster (or mask) in-memory has more information than on disk, we need to update it before checking equality + # The values in its .data.data that are masked in .data.mask are not necessarily equal to the nodata value + mask.data.data[mask.data.mask] = True # The default nodata 255 is converted to boolean True on masked values # Check all attributes are equal - assert all( - [ - np.ma.allequal(saved.data, mask.data), - saved.transform == mask.transform, - saved.crs == mask.crs, - saved.nodata == mask.nodata, - ] - ) + assert mask.raster_equal(saved) # Clean up temporary folder - fails on Windows try: From 4027c855357ae6e11e442bf7767bb2aaeaf39cd6 Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Thu, 1 Feb 2024 14:09:38 +0100 Subject: [PATCH 6/7] Linting --- geoutils/raster/raster.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/geoutils/raster/raster.py b/geoutils/raster/raster.py index f02434db..c664aae5 100644 --- a/geoutils/raster/raster.py +++ b/geoutils/raster/raster.py @@ -977,7 +977,7 @@ def __setitem__(self, index: Mask | NDArrayBool | Any, assign: NDArrayNum | Numb self._data[:, ind] = assign # type: ignore return None - def raster_equal(self, other: object) -> bool: + def raster_equal(self, other: RasterType) -> bool: """ Check if two rasters are equal. @@ -3605,7 +3605,6 @@ def __init__( # Convert masked array to boolean self._data = self.data.astype(bool) # type: ignore - def __repr__(self) -> str: """Convert mask to string representation.""" From 77e2c9ad672b8c0115cf78bf18d9b4c88a3e8846 Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Thu, 1 Feb 2024 14:12:37 +0100 Subject: [PATCH 7/7] Reset cache value and remove comment of other PR --- .github/workflows/python-tests.yml | 2 +- environment.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 62b8a00a..4c546cb1 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -48,7 +48,7 @@ jobs: path: ${{ env.CONDA }}/envs key: conda-${{ matrix.os }}-${{ matrix.python-version }}-${{ env.cache_date }}-${{ hashFiles('dev-environment.yml') }}-${{ env.CACHE_NUMBER }} env: - CACHE_NUMBER: 1 # Increase this value to reset cache if environment.yml has not changed + CACHE_NUMBER: 0 # Increase this value to reset cache if environment.yml has not changed id: cache # The trick below is necessary because the generic environment file does not specify a Python version, and ONLY diff --git a/environment.yml b/environment.yml index e0234412..1940594e 100644 --- a/environment.yml +++ b/environment.yml @@ -1,7 +1,7 @@ name: geoutils channels: - conda-forge -dependencies: # LOL +dependencies: - python>=3.9,<3.12 - geopandas>=0.12.0 - matplotlib