Fixes #5 and #7: contrast panel/levels histogram issues

PierreRaybaut · PierreRaybaut · commit 92e3d1cca22b · 2023-12-14T18:35:49.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,7 +6,9 @@
 
 * [Issue #3](https://github.com/PlotPyStack/PlotPy/issues/3) - `PlotWidget`: `ZeroDivisionError` on resize while ignoring constraints
 * [Issue #4](https://github.com/PlotPyStack/PlotPy/issues/4) - Average cross section: `RuntimeWarning: Mean of empty slice.`
+* [Issue #5](https://github.com/PlotPyStack/PlotPy/issues/5) - Contrast panel: levels histogram is sometimes not updated
 * [Issue #6](https://github.com/PlotPyStack/PlotPy/issues/6) - 1D Histogram items are not properly drawn
+* [Issue #7](https://github.com/PlotPyStack/PlotPy/issues/7) - Contrast panel: histogram may contains zeros periodically due to improper bin sizes
 
 ## Version 2.0.1 ##
 
diff --git a/plotpy/interfaces/items.py b/plotpy/interfaces/items.py
@@ -489,20 +489,24 @@ def can_sethistogram(self) -> bool:
 
 
 class IHistDataSource:
-    def get_histogram(self, nbins: int) -> tuple[np.ndarray, np.ndarray]:
+    def get_histogram(
+        self, nbins: int, drange: tuple[float, float] | None = None
+    ) -> tuple[np.ndarray, np.ndarray]:
         """
         Return a tuple (hist, bins) where hist is a list of histogram values
 
         Args:
-            nbins (int): number of bins
+            nbins: number of bins
+            drange: lower and upper range of the bins. If not provided, range is
+             simply (data.min(), data.max()). Values outside the range are ignored.
 
         Returns:
-            tuple: (hist, bins)
+            Tuple (hist, bins)
 
         Example of implementation:
 
-        def get_histogram(self, nbins):
+        def get_histogram(self, nbins, drange=None):
             data = self.get_data()
-            return np.histogram(data, nbins)
+            return np.histogram(data, bins=nbins, range=drange)
         """
         pass
diff --git a/plotpy/items/histogram.py b/plotpy/items/histogram.py
@@ -38,17 +38,21 @@ class HistDataSource:
     def __init__(self, data: np.ndarray) -> None:
         self.data = data
 
-    def get_histogram(self, nbins: int) -> tuple[np.ndarray, np.ndarray]:
+    def get_histogram(
+        self, nbins: int, drange: tuple[float, float] | None = None
+    ) -> tuple[np.ndarray, np.ndarray]:
         """
         Return a tuple (hist, bins) where hist is a list of histogram values
 
         Args:
-            nbins (int): number of bins
+            nbins: number of bins
+            drange: lower and upper range of the bins. If not provided, range is
+             simply (data.min(), data.max()). Values outside the range are ignored.
 
         Returns:
-            tuple: (hist, bins)
+            Tuple (hist, bins)
         """
-        return np.histogram(self.data, nbins)
+        return np.histogram(self.data, bins=nbins, range=drange)
 
 
 assert_interfaces_valid(HistDataSource)
@@ -73,6 +77,7 @@ def __init__(
         self.hist_count = None
         self.hist_bins = None
         self.bins = None
+        self.bin_range = None
         self.old_bins = None
         self.source: BaseImageItem | None = None
         self.logscale: bool | None = None
@@ -157,13 +162,30 @@ def get_bins(self) -> int | None:
         """
         return self.bins
 
+    def set_bin_range(self, bin_range: tuple[float, float] | None) -> None:
+        """Sets the range of the bins
+
+        Args:
+            bin_range: (min, max) or None for automatic range
+        """
+        self.bin_range = bin_range
+        self.update_histogram()
+
+    def get_bin_range(self) -> tuple[float, float] | None:
+        """Returns the range of the bins
+
+        Returns:
+            tuple: (min, max)
+        """
+        return self.bin_range
+
     def compute_histogram(self) -> tuple[np.ndarray, np.ndarray]:
         """Compute histogram data
 
         Returns:
             tuple: (hist, bins)
         """
-        return self.get_hist_source().get_histogram(self.bins)
+        return self.get_hist_source().get_histogram(self.bins, self.bin_range)
 
     def update_histogram(self) -> None:
         """Update histogram data"""
diff --git a/plotpy/items/image/base.py b/plotpy/items/image/base.py
@@ -946,22 +946,28 @@ def can_sethistogram(self) -> bool:
         """
         return False
 
-    def get_histogram(self, nbins: int) -> tuple[np.ndarray, np.ndarray]:
+    def get_histogram(
+        self, nbins: int, drange: tuple[float, float] | None = None
+    ) -> tuple[np.ndarray, np.ndarray]:
         """
         Return a tuple (hist, bins) where hist is a list of histogram values
 
         Args:
-            nbins (int): number of bins
+            nbins: number of bins
+            drange: lower and upper range of the bins. If not provided, range is
+             simply (data.min(), data.max()). Values outside the range are ignored.
 
         Returns:
-            tuple: (hist, bins)
+            Tuple (hist, bins)
         """
         if self.data is None:
             return [0], [0, 1]
         if self.histogram_cache is None or nbins != self.histogram_cache[0].shape[0]:
             if True:
                 # Note: np.histogram does not accept data with NaN
-                res = np.histogram(self.data[~np.isnan(self.data)], nbins)
+                res = np.histogram(
+                    self.data[~np.isnan(self.data)], bins=nbins, range=drange
+                )
             else:
                 # TODO: _histogram is faster, but caching is buggy in this version
                 _min, _max = get_nan_range(self.data)
diff --git a/plotpy/items/image/misc.py b/plotpy/items/image/misc.py
@@ -405,20 +405,26 @@ def can_sethistogram(self) -> bool:
         """
         return True
 
-    def get_histogram(self, nbins: int) -> tuple[np.ndarray, np.ndarray]:
+    def get_histogram(
+        self, nbins: int, drange: tuple[float, float] | None = None
+    ) -> tuple[np.ndarray, np.ndarray]:
         """
         Return a tuple (hist, bins) where hist is a list of histogram values
 
         Args:
-            nbins (int): number of bins
+            nbins: number of bins
+            drange: lower and upper range of the bins. If not provided, range is
+             simply (data.min(), data.max()). Values outside the range are ignored.
 
         Returns:
-            tuple: (hist, bins)
+            Tuple (hist, bins)
         """
         if self.data is None:
             return [0], [0, 1]
         _min, _max = get_nan_range(self.data)
-        if self.data.dtype in (np.float64, np.float32):
+        if drange is not None:
+            bins = np.linspace(drange[0], drange[1], nbins + 1)
+        elif self.data.dtype in (np.float64, np.float32):
             bins = np.unique(
                 np.array(np.linspace(_min, _max, nbins + 1), dtype=self.data.dtype)
             )
diff --git a/plotpy/panels/contrastadjustment.py b/plotpy/panels/contrastadjustment.py
@@ -19,6 +19,7 @@
 
 from typing import TYPE_CHECKING
 
+import numpy as np
 from guidata.configtools import get_icon, get_image_layout
 from guidata.dataset import DataSet, FloatItem
 from guidata.qthelpers import add_actions, create_action
@@ -37,7 +38,7 @@
 from plotpy.tools import AntiAliasingTool, BasePlotMenuTool, SelectPointTool, SelectTool
 
 if TYPE_CHECKING:  # pragma: no cover
-    from collections.abc import Callable
+    from collections.abc import Callable, Generator
 
     from qtpy.QtWidgets import QWidget
 
@@ -65,14 +66,10 @@ def __init__(self, parent: QWidget = None) -> None:
         self.antialiased = False
 
         # a dict of dict : plot -> selected items -> HistogramItem
-        self._tracked_items: dict[BasePlot, dict[BaseImageItem, CurveItem]] = {}
+        self._tracked_items: dict[BasePlot, dict[BaseImageItem, HistogramItem]] = {}
         self.param = CurveParam(_("Curve"), icon="curve.png")
         self.param.read_config(CONF, "histogram", "curve")
 
-        self.histparam = HistogramParam(_("Histogram"), icon="histogram.png")
-        self.histparam.logscale = False
-        self.histparam.n_bins = 256
-
         self.range = XRangeSelection(0, 1)
         self.range_mono_color = self.range.shapeparam.sel_line.color
         self.range_multi_color = CONF.get("histogram", "range/multi/color", "red")
@@ -103,11 +100,13 @@ def connect_plot(self, plot: BasePlot) -> None:
         plot.SIG_ITEM_REMOVED.connect(self.item_removed)
         plot.SIG_ACTIVE_ITEM_CHANGED.connect(self.active_item_changed)
 
-    def tracked_items_gen(self) -> tuple[BaseImageItem, CurveItem]:
+    def tracked_items_gen(
+        self,
+    ) -> Generator[tuple[BaseImageItem, HistogramItem], None, None]:
         """Generator of tracked items"""
-        for plot, items in list(self._tracked_items.items()):
-            for item in list(items.items()):
-                yield item  # tuple item,curve
+        for _plot, items in list(self._tracked_items.items()):
+            for item_curve_tuple in list(items.items()):
+                yield item_curve_tuple  # tuple item,curve
 
     def __del_known_items(self, known_items: dict, items: list) -> None:
         """Delete known items
@@ -129,7 +128,9 @@ def selection_changed(self, plot: BasePlot) -> None:
         Args:
             plot: plot whose selection changed
         """
-        items = plot.get_selected_items(item_type=IVoiImageItemType)
+        items: list[BaseImageItem] = plot.get_selected_items(
+            item_type=IVoiImageItemType
+        )
         known_items = self._tracked_items.setdefault(plot, {})
 
         if items:
@@ -153,13 +154,10 @@ def selection_changed(self, plot: BasePlot) -> None:
 
         for item in items:
             if item not in known_items:
-                imin, imax = item.get_lut_range_full()
-                delta = int(float(imax) - float(imin))
-                if delta > 0 and delta < 256:
-                    self.histparam.n_bins = delta
-                else:
-                    self.histparam.n_bins = 256
-                curve = HistogramItem(self.param, self.histparam, keep_weakref=True)
+                histparam = HistogramParam(_("Histogram"), icon="histogram.png")
+                histparam.logscale = False
+                histparam.n_bins = 256
+                curve = HistogramItem(self.param, histparam, keep_weakref=True)
                 curve.set_hist_source(item)
                 self.add_item(curve, z=0)
                 known_items[item] = curve
@@ -170,8 +168,18 @@ def selection_changed(self, plot: BasePlot) -> None:
             return
         self.param.shade = 1.0 / nb_selected
         for item, curve in self.tracked_items_gen():
+            if np.issubdtype(item.data.dtype, np.integer):
+                # For integer data, we use the full range of data type
+                info = np.iinfo(item.data.dtype)
+                curve.histparam.bin_min = info.min
+                curve.histparam.bin_max = info.max
+                curve.histparam.n_bins = min(info.max - info.min + 1, 256)
+            else:
+                curve.histparam.bin_min = None
+                curve.histparam.bin_max = None
+                curve.histparam.n_bins = 256
             self.param.update_item(curve)
-            self.histparam.update_hist(curve)
+            curve.histparam.update_hist(curve)
 
         self.active_item_changed(plot)
 
diff --git a/plotpy/styles/histogram.py b/plotpy/styles/histogram.py
@@ -1,10 +1,15 @@
 # -*- coding: utf-8 -*-
 
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
 from guidata.dataset import (
     BoolItem,
     ChoiceItem,
     ColorItem,
     DataSet,
+    FloatItem,
     GetAttrProp,
     IntItem,
     StringItem,
@@ -14,26 +19,39 @@
 from plotpy.styles.base import ItemParameters
 from plotpy.styles.image import BaseImageParam
 
+if TYPE_CHECKING:  # pragma: no cover
+    from plotpy.items import Histogram2DItem, HistogramItem
+
 
 class HistogramParam(DataSet):
     n_bins = IntItem(_("Bins"), default=100, min=1, help=_("Number of bins"))
+    bin_min = FloatItem(_("Min"), default=None, help=_("Minimum value"), check=False)
+    bin_max = FloatItem(_("Max"), default=None, help=_("Maximum value"), check=False)
     logscale = BoolItem(_("logarithmic"), _("Y-axis scale"), default=False)
 
-    def update_param(self, obj):
-        """
+    def update_param(self, item: HistogramItem) -> None:
+        """Update the histogram parameters from the plot item
 
-        :param obj:
+        Args:
+            item: Histogram item
         """
-        self.n_bins = obj.get_bins()
-        self.logscale = obj.get_logscale()
+        self.n_bins = item.get_bins()
+        self.bin_min, self.bin_max = item.get_bin_range()
+        self.logscale = item.get_logscale()
 
-    def update_hist(self, hist):
-        """
+    def update_hist(self, item: HistogramItem) -> None:
+        """Update the histogram plot item from the parameters
 
-        :param hist:
+        Args:
+            item: Histogram item
         """
-        hist.set_bins(self.n_bins)
-        hist.set_logscale(self.logscale)
+        if self.bin_min is None or self.bin_max is None:
+            item.bin_range = None
+        else:
+            item.bin_range = (self.bin_min, self.bin_max)
+        item.bins = self.n_bins
+        item.logscale = self.logscale
+        item.update_histogram()
 
 
 class Histogram2DParam(BaseImageParam):
@@ -79,24 +97,26 @@ class Histogram2DParam(BaseImageParam):
         help=_("Background color when no data is present"),
     )
 
-    def update_param(self, obj):
-        """
+    def update_param(self, item: Histogram2DItem) -> None:
+        """Update the histogram parameters from the plot item
 
-        :param obj:
+        Args:
+            item: 2D Histogram item
         """
-        super().update_param(obj)
-        self.logscale = obj.logscale
-        self.nx_bins, self.ny_bins = obj.nx_bins, obj.ny_bins
+        super().update_param(item)
+        self.logscale = item.logscale
+        self.nx_bins, self.ny_bins = item.nx_bins, item.ny_bins
 
-    def update_histogram(self, histogram):
-        """
+    def update_histogram(self, item: Histogram2DItem) -> None:
+        """Update the histogram plot item from the parameters
 
-        :param histogram:
+        Args:
+            item: 2D Histogram item
         """
-        histogram.logscale = int(self.logscale)
-        histogram.set_background_color(self.background)
-        histogram.set_bins(self.nx_bins, self.ny_bins)
-        self.update_item(histogram)
+        item.logscale = int(self.logscale)
+        item.set_background_color(self.background)
+        item.set_bins(self.nx_bins, self.ny_bins)
+        self.update_item(item)
 
 
 class Histogram2DParam_MS(Histogram2DParam):
diff --git a/plotpy/tests/widgets/test_simple_window.py b/plotpy/tests/widgets/test_simple_window.py
