Repository: DigitalSlideArchive/HistomicsTK (python)

Issue:
Potential bug in separate_stains_macenko_pca.py | Function: argpercentile
Hi!

First off, thanks to all developers and maintainers for this amazing package!

I am using HistomicsTK on Ubuntu 20.04.5 LTS.

During my work with it I noticed a potential bug in the file `separate_stains_macenko_pca.py' in the function argpercentile.

https://github.com/DigitalSlideArchive/HistomicsTK/blob/15262df548a2579846a0dcfd8b7f41e30ddff58e/histomicstk/preprocessing/color_deconvolution/separate_stains_macenko_pca.py#L121C1-L125C41

For instance, this function is used in line 98 with a value for p = 0.99:
https://github.com/DigitalSlideArchive/HistomicsTK/blob/4748f9d2e00216a59056c1a75c87f65dc6b271aa/histomicstk/preprocessing/color_deconvolution/separate_stains_macenko_pca.py#L98C4-L98C56

With p = 0.99, as soon as the size of the provided array is smaller than 51, the calculated index will be larger than the length of the array. This leads to an "out of bounds" error in the subsequent call of numpy.argpartition(arr, i)[i] on line 125.

arr.size = 50, p = 0.99
i = 50 -> Will cause an error

I guess the easiest fix would be something like this:
```
i = int(p * arr.size + 0.5)
if i >= arr.size:
    i -= 1
return numpy.argpartition(arr, i)[i]
```

Having said all of this, I honestly am not sure if this combination of a small input array and a high percentage value is not supposed to occur in a normal application flow. Therefore, I am uncertain if this would merely mask another underlying problem.

Thank you for your time!


Pull request:
Macenko pca out of bounds fix
fixes #1006 

Patch:
diff --git a/histomicstk/preprocessing/color_deconvolution/separate_stains_macenko_pca.py b/histomicstk/preprocessing/color_deconvolution/separate_stains_macenko_pca.py
index 5462b58b..0fb00d88 100644
--- a/histomicstk/preprocessing/color_deconvolution/separate_stains_macenko_pca.py
+++ b/histomicstk/preprocessing/color_deconvolution/separate_stains_macenko_pca.py
@@ -121,5 +121,5 @@ def _get_angles(m):
 def argpercentile(arr, p):
     """Calculate index in arr of element nearest the pth percentile."""
     # Index corresponding to percentile
-    i = int(p * arr.size + 0.5)
+    i = min(int(p * arr.size + 0.5), arr.size - 1)
     return numpy.argpartition(arr, i)[i]


Test patch:
diff --git a/tests/test_color_deconvolution.py b/tests/test_color_deconvolution.py
index 2de55c9d..4628e8ab 100644
--- a/tests/test_color_deconvolution.py
+++ b/tests/test_color_deconvolution.py
@@ -2,6 +2,8 @@ import numpy as np
 import skimage.io
 
 from histomicstk.preprocessing import color_deconvolution as htk_dcv
+from histomicstk.preprocessing.color_deconvolution.separate_stains_macenko_pca import \
+    argpercentile as htk_ap
 
 from .datastore import datastore
 
@@ -20,6 +22,29 @@ class TestMacenko:
 
         np.testing.assert_allclose(w, w_expected, atol=1e-6)
 
+    def test_argpercentile(self):
+        arr = np.array([])
+        with np.testing.assert_raises(IndexError):
+            htk_ap(arr, 0.5)
+
+        arr = np.array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10])
+
+        w = htk_ap(arr, 0.5)
+        w_expected = 5
+        np.testing.assert_equal(w, w_expected)
+
+        w = htk_ap(arr, 0.1)
+        w_expected = 1
+        np.testing.assert_equal(w, w_expected)
+
+        w = htk_ap(arr, 0.0)
+        w_expected = 0
+        np.testing.assert_equal(w, w_expected)
+
+        w = htk_ap(arr, 0.99)
+        w_expected = 9
+        np.testing.assert_equal(w, w_expected)
+
 
 class TestColorDeconvolution: