FIX: fixed compare showing all cells white and 0 as maximum diff when all the cells with differences are 0 in the first array (fixes #288)

Author: gdementen

doc/source/changes/version_0_35.rst.inc

@@ -117,4 +117,7 @@ Fixes
 * fixed :py:obj:`compare()` colors when the only difference is nans on either
   side.
 
+* fixed :py:obj:`compare()` colors and max difference label when the only
+  differences are for rows where the value is 0 in the first array.
+
 * fixed single column plot in viewer when ticks are not strings

larray_editor/comparator.py

@@ -5,7 +5,7 @@
 from qtpy.QtWidgets import (QWidget, QVBoxLayout, QListWidget, QSplitter, QHBoxLayout,
                             QLabel, QCheckBox, QLineEdit, QComboBox, QMessageBox)
 
-from larray_editor.utils import replace_inf, _
+from larray_editor.utils import _
 from larray_editor.arraywidget import ArrayEditorWidget
 from larray_editor.editor import AbstractEditorWindow, DISPLAY_IN_GRID
 
@@ -112,7 +112,8 @@ def _get_atol_rtol(self):
             self.tolerance_line_edit.setText('')
             tol = 0
             QMessageBox.critical(self, "Error", str(e))
-        return (tol, 0) if self.tolerance_combobox.currentText() == "absolute" else (0, tol)
+        is_absolute = self.tolerance_combobox.currentText() == "absolute"
+        return (tol, 0) if is_absolute else (0, tol)
 
     # override keyPressEvent to prevent pressing Enter after changing the tolerance value
     # in associated QLineEdit to close the parent dialog box
@@ -160,31 +161,69 @@ def _update_from_combined_array(self):
 
         atol, rtol = self._get_atol_rtol()
         try:
-            self._diff_below_tolerance = self._combined_array.eq(self._array0, rtol=rtol, atol=atol, nans_equal=self.nans_equal)
+            # eq does not take atol and rtol into account
+            eq = self._combined_array.eq(self._array0,
+                                         nans_equal=self.nans_equal)
+            isclose = self._combined_array.eq(self._array0,
+                                              rtol=rtol, atol=atol,
+                                              nans_equal=self.nans_equal)
         except TypeError:
-            self._diff_below_tolerance = self._combined_array == self._array0
+            # object arrays
+            eq = self._combined_array == self._array0
+            isclose = eq
+        self._diff_below_tolerance = isclose
 
         try:
             with np.errstate(divide='ignore', invalid='ignore'):
                 diff = self._combined_array - self._array0
                 reldiff = diff / self._array0
-
+            # make reldiff 0 where the values are the same than array0 even for
+            # special values (0, nan, inf, -inf)
+            # at this point reldiff can still contain nan and infs
+            reldiff = la.where(eq, 0, reldiff)
+
+            # 1) compute maxabsreldiff for the label
+            #    this should NOT exclude nans or infs
+            relmin = reldiff.min(skipna=False)
+            relmax = reldiff.max(skipna=False)
+            maxabsreldiff = max(abs(relmin), abs(relmax))
+
+            # 2) compute bg_value
             # replace -inf by min(reldiff), +inf by max(reldiff)
-            finite_reldiff, finite_relmin, finite_relmax = replace_inf(reldiff)
-            maxabsreldiff = max(abs(finite_relmin), abs(finite_relmax))
-
-            # We need a separate version for bg and the label, so that when we modify atol/rtol, the background
-            # color is updated but not the maxreldiff label
-            # this is necessary for nan, inf and -inf (because inf - inf = nan, not 0)
-            # this is more precise than divnot0, it only ignore 0 / 0, not x / 0
-            reldiff_for_bg = la.where(self._diff_below_tolerance, 0, finite_reldiff)
-            maxabsreldiff_for_bg = max(abs(np.nanmin(reldiff_for_bg)), abs(np.nanmax(reldiff_for_bg)))
+            reldiff_for_bg = reldiff.copy()
+            isneginf = reldiff == -np.inf
+            isposinf = reldiff == np.inf
+            isinf = isneginf | isposinf
+
+            # given the way reldiff is constructed, it cannot contain only infs
+            # (because inf/inf is nan) it can contain only infs and nans though,
+            # in which case finite_relXXX will be nan, so unless the array
+            # is empty, finite_relXXX should never be inf
+            finite_relmin = np.nanmin(reldiff, where=~isinf, initial=np.inf)
+            finite_relmax = np.nanmax(reldiff, where=~isinf, initial=-np.inf)
+            # special case when reldiff contains only 0 and infs (to avoid
+            # coloring the inf cells white in that case)
+            if finite_relmin == 0 and finite_relmax == 0 and isinf.any():
+                finite_relmin = -1
+                finite_relmax = 1
+            reldiff_for_bg[isneginf] = finite_relmin
+            reldiff_for_bg[isposinf] = finite_relmax
+
+            # make sure that "acceptable" differences show as white
+            reldiff_for_bg = la.where(isclose, 0, reldiff_for_bg)
+
+            # We need a separate version for bg and the label, so that when we
+            # modify atol/rtol, the background color is updated but not the
+            # maxreldiff label
+            maxabsreldiff_for_bg = max(abs(np.nanmin(reldiff_for_bg)),
+                                       abs(np.nanmax(reldiff_for_bg)))
             if maxabsreldiff_for_bg:
                 # scale reldiff to range 0-1 with 0.5 for reldiff = 0
                 self._bg_value = (reldiff_for_bg / maxabsreldiff_for_bg) / 2 + 0.5
             # if the only differences are nans on either side
-            elif not self._diff_below_tolerance.all():
-                # use white (0.5) everywhere except where reldiff is nan, so that nans are grey
+            elif not isclose.all():
+                # use white (0.5) everywhere except where reldiff is nan, so
+                # that nans are grey
                 self._bg_value = reldiff_for_bg + 0.5
             else:
                 # do NOT use full_like as we don't want to inherit array dtype

larray_editor/tests/test_api_larray.py

@@ -274,14 +274,39 @@ def test_plot_returning_ax_and_using_show():
 # arr1['M'] = la.nan
 # compare(arr1, arr2, nans_equal=False, title='with nans on left side')
 # arr2['M'] = la.nan
-# compare(arr1, arr2, nans_equal=False, title='with nans on both sides')
+# compare(arr1, arr2, title='with nans on both sides (nans_equal=True)')
+# compare(arr1, arr2, nans_equal=False, title='with nans on both sides (nans_equal=False)')
 # arr1['M'] = arr1_m
 # compare(arr1, arr2, nans_equal=False, title='with nans on right side')
 #
 # arr1 = la.ndtest((3, 3))
 # arr2 = 2 * arr1
 # arr3 = la.where(arr1 % 2 == 0, arr1, -arr1)
-# compare(arr1, arr2, arr3, bg_gradient='blue-white-red')
+# compare(arr1, arr2, arr3, bg_gradient='blue-white-red', title='changed gradient')
+
+# arr1 = la.ndtest(3).astype(np.float64)
+# arr2 = arr1.copy()
+# arr2['a0'] = 42
+# arr3 = arr2.copy()
+# arr3['a1'] = 4
+# arr4 = arr1.copy()
+# arr4['a1'] = la.inf
+# arr5 = arr1.copy()
+# arr5['a1'] = -la.inf
+# arr6 = arr1.copy()
+# arr6['a1'] = 1.00000000001
+# compare(arr1, arr2)
+# compare(arr2, arr1)
+# compare(arr1, arr1)
+# compare(arr1, arr3)
+# compare(arr3, arr1)
+# compare(arr1, arr4)
+# compare(arr4, arr1)
+# compare(arr1, arr5)
+# compare(arr5, arr1)
+# compare(arr5, arr5)
+# compare(arr1, arr6)
+# compare(arr6, arr1)
 
 # test for arr.plot(show=True) which is the default
 # =================================================

larray_editor/utils.py

@@ -382,40 +382,6 @@ def show_figure(figure, title=None, parent=None):
     window.show()
 
 
-def replace_inf(value):
-    """Replace -inf/+inf in array with respectively min(array_without_inf)/max(array_without_inf).
-
-    It leaves nans intact.
-
-    Parameters
-    ----------
-    value : np.ndarray or any compatible type
-        Input array.
-
-    Returns
-    -------
-    (np.ndarray, float, float)
-        array with infinite values replaced by the min and maximum respectively
-        minimum finite value
-        maximum finite value
-
-    Examples
-    --------
-    >>> replace_inf(np.array([-5, np.inf, 0, -np.inf, -4, np.nan, 5]))
-    (array([ -5.,   5.,   0.,  -5.,  -4.,  nan,   5.]), -5.0, 5.0)
-    """
-    value = value.copy()
-    # replace -inf by min(value)
-    isneginf = value == -np.inf
-    minvalue = np.nanmin(value[~isneginf])
-    value[isneginf] = minvalue
-    # replace +inf by max(value)
-    isposinf = value == np.inf
-    maxvalue = np.nanmax(value[~isposinf])
-    value[isposinf] = maxvalue
-    return value, minvalue, maxvalue
-
-
 def scale_to_01range(value, vmin, vmax):
     """Scale value to 0-1 range based on vmin and vmax.