Add reclassify raster #197

lehtonenp · 2023-10-10T11:10:35Z

This PR for reclassifying raster data functions.

…pendency

… in geometrical intervals

…ta in equal intervals

…e logic with the small array

lehtonenp · 2024-02-08T13:21:46Z

@nialov, I have made the requested changes to the reclassify methods and tests. Is there anything more to add?

…ocal-morans-i

nialov

Looks good, only few small things:

Remove tests/data/small_raster.tif.aux.xml file. It was probably just added by opening the raster in e.g. ArcGIS or QGIS.
Check that when you use the masked array you do not use np.nan... methods such as np.nanmax. Instead use np.ma.max and others from np.ma.

After these, LGTM!

nialov · 2024-02-12T09:10:53Z

eis_toolkit/raster_processing/reclassify_raster.py

+    mask = band == nan_value
+    masked_array = np.ma.masked_array(data=band, mask=mask)
+
+    median_value = np.nanmedian(masked_array)


Suggested change

median_value = np.nanmedian(masked_array)

median_value = np.ma.median(masked_array)

nialov · 2024-02-12T09:11:50Z

eis_toolkit/raster_processing/reclassify_raster.py

+    min_tail = np.nanmin(tail_values)
+    max_tail = np.nanmax(tail_values)


Suggested change

min_tail = np.nanmin(tail_values)

max_tail = np.nanmax(tail_values)

min_tail = np.ma.min(tail_values)

max_tail = np.ma.max(tail_values)

nialov · 2024-02-12T09:14:50Z

eis_toolkit/raster_processing/reclassify_raster.py

+from eis_toolkit.exceptions import InvalidParameterValueException
+
+
+def _bands_non_negative(band: Sequence):


Suggested change

def _bands_non_negative(band: Sequence):

def _check_bands_non_negative(band: Sequence):

…versions

…ctions

nmaarnio

Hey, I took a quick look at this and wrote some comments. I would consider renaming this module to "reclassify.py" and the tool names to reclassify_<strategy> or reclassify_with_<strategy>. Any thoughts about these renamings @lehtonenp and @nialov ?

nmaarnio · 2024-02-15T11:44:30Z

eis_toolkit/raster_processing/reclassify_raster.py

+def _check_bands_non_negative(band: Sequence):
+    if any(n < 0 for n in band):
+        raise InvalidParameterValueException("The list bands contains negative values.")


I recommend taking this out and just using check_raster_bands found in utilities -> checks -> raster module.

nmaarnio · 2024-02-15T11:45:25Z

eis_toolkit/raster_processing/reclassify_raster.py

+def raster_with_manual_breaks(  # type: ignore[no-any-unimported]
+    raster: rasterio.io.DatasetReader,
+    breaks: Sequence[int],
+    bands: Sequence[int],


This does not have any default value even thought the docstring claims so.

Suggested change

bands: Sequence[int],

bands: Optional[Sequence[int]] = None,

This applies to all tools in this module.

nmaarnio · 2024-02-15T11:50:16Z

eis_toolkit/raster_processing/reclassify_raster.py

+
+@beartype
+def raster_with_geometrical_intervals(  # type: ignore[no-any-unimported]
+    raster: rasterio.io.DatasetReader, number_of_classes: int, nan_value: Union[int, float], bands: Sequence[int]


Because the input is a raster DatasetReader, I don't think a nan_value parameter is strictly necessary. The raster should have nodata value defined in its metadata/profile and that should be used in the mask. But if we want to let the user to work with a faulty raster and override nodata in these kind tools, I would atleast change the name to nodata_value and the type Number (imported like this: from numbers import Number) which we have used for cases that allow floats and ints.

eis_toolkit/raster_processing/reclassify_raster.py

lehtonenp · 2024-02-15T12:33:06Z

Hey, I took a quick look at this and wrote some comments. I would consider renaming this module to "reclassify.py" and the tool names to reclassify_<strategy> or reclassify_with_<strategy>. Any thoughts about these renamings @lehtonenp and @nialov ?

@nmaarnio, reclassify_with_<strategy> is a suitable name. I can change the function naming.

nmaarnio

LGTM now!

Since @nialov was the main reviewer of this PR, let's wait for his approval before merging.

A small note not regarding the implementation itself:I guess the original comment of this PR that "It is not to be merged to the EIS toolkit code base." could be removed at this point

nialov

LGTM! Very well coded functionality!

nmaarnio · 2024-02-21T10:37:39Z

I actually encountered a few issues when implementing CLI functions for these tools just now. check_raster_bands expects the bands parameter to be a Sequence. Now None can be delivered to it. Furthermore, I realized that the functions are returning sequence of 2D Numpy arrays, whereas a 3D Numpy array is how mulitband raster data is usually represented (and can be easily saved with Rasterio).

I ended up creating a suggestion for refactored version of the public relcassification functions. Here cases where bands is None or empty sequence are handled, 3D Numpy array allocated and returned. Feel free to modify this or disagree, I just crafted it while thinking of these issues!

@beartype
def reclassify_with_manual_breaks(  # type: ignore[no-any-unimported]
    raster: rasterio.io.DatasetReader,
    breaks: Sequence[int],
    bands: Optional[Sequence[int]] = None,
) -> Tuple[np.ndarray, dict]:
    """Classify raster with manual breaks.

    If bands are not given, all bands are used for classification.

    Args:
        raster: Raster to be classified.
        breaks: List of break values for the classification.
        bands: Selected bands from multiband raster. Indexing begins from one. Defaults to None.

    Returns:
        Raster classified with manual breaks and metadata.

    Raises:
        InvalidParameterValueException: Bands contain negative values.
    """

    if bands is None or len(bands) == 0:
        bands = range(1, raster.count + 1)
    else:
        check_raster_bands(raster, bands)

    out_image = np.empty((len(bands), raster.height, raster.width))
    out_meta = raster.meta.copy()

    for i, band in enumerate(bands):
        band_data = raster.read(band)
        out_image[i] = _reclassify_with_manual_breaks(band_data, breaks)

    return out_image, out_meta

…rding to feedback

…nto add-reclassify-raster

nmaarnio · 2024-02-22T06:42:33Z

Some changes to poetry.lock and pyproject.toml had slipped to this PR in some merge, I removed them. Looks good for merging now!

lehtonenp and others added 11 commits December 16, 2022 12:55

add new file

ea1d7f2

change metadata of rasterfile

8aa2b3f

initial functionality for intervals

8565a08

test functions with mc.load_example()

c89d978

refactor: change input raster mode, linting and tests

e086d22

fix equal interval test

4f0c6f9

documentation

79fc7a9

adjust geometrical intervals from Torppa's comments, remove unused de…

c509001

…pendency

include path_to_file variable to functions, use it fora temp file

c67556a

fix geometrical intervals

8ab5af1

Add reclassify raster functions and initial tests

b898967

lehtonenp requested a review from nmaarnio October 10, 2023 11:10

lehtonenp added 18 commits November 1, 2023 08:50

Merge to master

d9eebf8

Move reclassify raster tests to correct folder

5e86d6f

Update dependencies

40a2218

Add path_to_file variable to private functions, fix np.where() clause…

25b4c65

… in geometrical intervals

Clean up reclassify raster test file

c95b76c

Clean up reclassify raster test file

a5b3223

Commit pre-commit changes

76b46dd

Commit pre-commit changes

a8f88ed

Merge branch 'master' into add-reclassify-raster

e0da170

Change author of geometrical intervals

4abf4ff

Rework standard deviation classification

2a8b4e7

Change natural breaks test text, exlucde pre-commit

4bdeeba

Change natural breaks test text

14d27da

Unify reading raster in tests

d337b0e

Modify code comments, use np.nanmin & np.nanmax to get values from da…

df5d533

…ta in equal intervals

Change linspace parameters back to 0 and 100

9333ac2

Add non zero check to geometrical intervals

c072f7d

Add small numpy array as test data, test the functionality of the cor…

e3d4cf2

…e logic with the small array

Pre commit fixes

181952f

Merge branch '287-Add-Local-Morans-I' into 319-add-cli-function-for-l…

2799b95

…ocal-morans-i

nialov requested changes Feb 12, 2024

View reviewed changes

nmaarnio and others added 8 commits February 12, 2024 12:14

Locked Scipy to 1.11.4 due to failing imports from libpysal on other …

3ce8c73

…versions

Added CLI function for Local Moran's I

3f3fe8e

Rename negative band check function, change .nan functions to .ma fun…

dca3786

…ctions

Remove small_raster.tif.aux.xml

ae06a07

Fix typo in Geometrical intervals: .ma.nmin -> .ma.min

6ea9680

Fix expected_output to reflect changes in the function

9cc41c5

Fix expected_output to reflect expected output in conda env

7ba7a60

Fix expected_output to reflect expected output in conda env

94ae080

nmaarnio requested changes Feb 15, 2024

View reviewed changes

lehtonenp added 4 commits February 15, 2024 14:51

Pre-commit changes

fb04051

Pre-commit changes

bbd377f

Pre-commit changes

3b26d99

Update docs for reclassify

481a3b6

nmaarnio approved these changes Feb 21, 2024

View reviewed changes

nialov approved these changes Feb 21, 2024

View reviewed changes

lehtonenp added 2 commits February 21, 2024 11:24

Merge to master

aeccd0b

Merge master to add-reclassify-raster

7b7f8be

lehtonenp and others added 4 commits February 21, 2024 14:41

Change output type of public functions, improve public functions acco…

73a1f7b

…rding to feedback

Remove unused import

da8877e

Merge branch 'master' of https://github.com/GispoCoding/eis_toolkit i…

bee4434

…nto add-reclassify-raster

Match files with master

bf2fb70

nmaarnio merged commit 43f8919 into master Feb 22, 2024
6 checks passed

nmaarnio deleted the add-reclassify-raster branch February 22, 2024 06:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add reclassify raster #197

Add reclassify raster #197

lehtonenp commented Oct 10, 2023 •

edited

Loading

lehtonenp commented Feb 8, 2024

nialov left a comment •

edited

Loading

nialov Feb 12, 2024

nialov Feb 12, 2024

nialov Feb 12, 2024

nmaarnio left a comment

nmaarnio Feb 15, 2024

nmaarnio Feb 15, 2024

nmaarnio Feb 15, 2024

lehtonenp commented Feb 15, 2024 •

edited

Loading

nmaarnio left a comment •

edited

Loading

nialov left a comment

nmaarnio commented Feb 21, 2024

nmaarnio commented Feb 22, 2024

	median_value = np.nanmedian(masked_array)
	median_value = np.ma.median(masked_array)

		min_tail = np.nanmin(tail_values)
		max_tail = np.nanmax(tail_values)

		from eis_toolkit.exceptions import InvalidParameterValueException


		def _bands_non_negative(band: Sequence):

	def _bands_non_negative(band: Sequence):
	def _check_bands_non_negative(band: Sequence):

Add reclassify raster #197

Add reclassify raster #197

Conversation

lehtonenp commented Oct 10, 2023 • edited Loading

lehtonenp commented Feb 8, 2024

nialov left a comment • edited Loading

Choose a reason for hiding this comment

nialov Feb 12, 2024

Choose a reason for hiding this comment

nialov Feb 12, 2024

Choose a reason for hiding this comment

nialov Feb 12, 2024

Choose a reason for hiding this comment

nmaarnio left a comment

Choose a reason for hiding this comment

nmaarnio Feb 15, 2024

Choose a reason for hiding this comment

nmaarnio Feb 15, 2024

Choose a reason for hiding this comment

nmaarnio Feb 15, 2024

Choose a reason for hiding this comment

lehtonenp commented Feb 15, 2024 • edited Loading

nmaarnio left a comment • edited Loading

Choose a reason for hiding this comment

nialov left a comment

Choose a reason for hiding this comment

nmaarnio commented Feb 21, 2024

nmaarnio commented Feb 22, 2024

lehtonenp commented Oct 10, 2023 •

edited

Loading

nialov left a comment •

edited

Loading

lehtonenp commented Feb 15, 2024 •

edited

Loading

nmaarnio left a comment •

edited

Loading