Fix OverflowError in the curve fitting dialog when a fitted value is outside the slider range

.github/workflows/test_pyqt5.yml

@@ -18,7 +18,8 @@ jobs:
     env:
       DISPLAY: ':99.0'
 
-    runs-on: ubuntu-latest
+    # Python 3.9 is not available on newer Ubuntu runners (ubuntu-latest)
+    runs-on: ubuntu-24.04
     strategy:
       fail-fast: false
       matrix:

doc/release_notes/release_2.11.md

@@ -1,5 +1,11 @@
 # Version 2.11 #
 
+## PlotPy Version 2.11.2 ##
+
+🛠️ Bug fixes:
+
+* **Curve fitting dialog** — Running the automatic fit no longer raises `OverflowError: argument 1 overflowed` errors when a fitted value ends up far outside the range of its slider (e.g. with the Powell method on a fit range restricted to the tail of a peak). These errors left the slider of the parameter unresponsive, and the values of the following parameters were not displayed. Such values are now shown with a warning icon, whose tooltip gives the slider range, while the slider stays at its nearest end. Typing `nan` or `inf` in a parameter field now restores the previous value instead of raising errors, and the number of slider steps must be at least 2 in the parameter properties dialog (closes [Issue #76](https://github.com/PlotPyStack/PlotPy/issues/76))
+
 ## PlotPy Version 2.11.1 ##
 
 🛠️ Bug fixes:

plotpy/locale/fr/LC_MESSAGES/plotpy.po

@@ -1628,6 +1628,10 @@ msgstr "Paramètre d'ajustement"
 msgid "Edit '{name}' fit parameter properties"
 msgstr "Modifier les propriétés du paramètre d'ajustement '{name}'"
 
+#, python-brace-format
+msgid "This value is outside the slider range ({vmin} to {vmax})"
+msgstr "Cette valeur est en dehors de la plage du curseur ({vmin} à {vmax})"
+
 msgid "Fit parameters"
 msgstr "Paramètres d'ajustement"
 

plotpy/tests/unit/test_fit_slider.py

@@ -0,0 +1,143 @@
+# -*- coding: utf-8 -*-
+#
+# Licensed under the terms of the BSD 3-Clause
+# (see plotpy/LICENSE for details)
+
+"""Curve fitting parameter sliders: values outside the slider range, non-finite
+values and invalid inputs keep the fit dialog usable."""
+
+from __future__ import annotations
+
+import numpy as np
+import pytest
+from guidata.qthelpers import qt_app_context
+
+from plotpy.widgets.fit import FitDialog, FitParam, FitParamDataSet
+
+
+def _param(
+    value: float = 0.5, vmin: float = 0.0, vmax: float = 1.0, logscale: bool = False
+) -> FitParam:
+    """Return a fit parameter with its widgets, updated for the given value"""
+    param = FitParam("p", 0.5, 0.0, 1.0, logscale=logscale)
+    param.create_widgets(None, lambda: None)
+    param.min, param.max, param.value = vmin, vmax, value
+    param.update()
+    return param
+
+
+def _has_range_warning(param: FitParam) -> bool:
+    """Return True if the parameter shows the out-of-range warning"""
+    return param.range_warning.isVisible() and bool(param.lineedit.toolTip())
+
+
+@pytest.mark.parametrize("logscale", [False, True])
+@pytest.mark.parametrize("value", [1e15, 2.0, -1.0, -1e15])
+def test_value_outside_range_clamps_slider(value: float, logscale: bool):
+    """The slider stays at the nearest end and a warning is shown."""
+    with qt_app_context(exec_loop=False):
+        param = _param(value, logscale=logscale)
+        expected = param.steps - 1 if value > param.max else 0
+        assert param.slider.value() == expected
+        assert not param.slider.signalsBlocked()
+        assert param.value == value
+        assert _has_range_warning(param)
+        param.value = 0.5
+        param.update()
+        assert not _has_range_warning(param)
+
+
+@pytest.mark.parametrize(
+    "value, vmin, vmax",
+    [
+        (np.nan, 0.0, 1.0),
+        (np.inf, 0.0, 1.0),
+        (-np.inf, 0.0, 1.0),
+        (2.0, 1.0, 1.0),
+        (np.float64(2.0), 1.0, 1.0),
+        (1e308, -1e308, 1e308),
+    ],
+)
+def test_undefined_slider_position(value: float, vmin: float, vmax: float):
+    """Non-finite values and empty ranges leave the slider position unchanged."""
+    with qt_app_context(exec_loop=False):
+        param = _param(0.5)
+        position = param.slider.value()
+        param.min, param.max, param.value = vmin, vmax, value
+        param.update()
+        assert param.slider.value() == position
+        assert not param.slider.signalsBlocked()
+
+
+@pytest.mark.parametrize("logscale", [False, True])
+def test_slider_mapping_round_trip(logscale: bool):
+    """Slider position -> value -> slider position gives the same position."""
+    with qt_app_context(exec_loop=False):
+        param = _param(0.5, 0.0, 10.0, logscale)
+        last = param.steps - 1
+        for position in (0, 1, 1234, last - 1, last):
+            param.slider_value_changed(position)
+            param.update_slider_value()
+            assert param.slider.value() == position
+        param.value = param.min
+        param.update_slider_value()
+        assert param.slider.value() == 0
+        param.value = param.max
+        param.update_slider_value()
+        assert param.slider.value() == last
+
+
+def test_autofit_with_value_far_outside_range():
+    """All parameters are updated when the fit leaves a slider range."""
+    x = np.linspace(0.0, 1.0, 50)
+    y = 1e6 * x + 3.0
+
+    def fitfunc(x: np.ndarray, params: list[float]) -> np.ndarray:
+        a, b = params
+        return a * x + b
+
+    with qt_app_context(exec_loop=False):
+        # The fitted slope is 1e6 times the slider range width: its slider position
+        # used to exceed the 32-bit integer range accepted by `QSlider.setValue`
+        params = [FitParam("a", 0.5, 0.0, 1.0), FitParam("b", 0.0, -5.0, 5.0)]
+        dialog = FitDialog(edit=True, auto_fit=True)
+        dialog.set_data(x, y, fitfunc, params)
+        dialog.autofit()
+        a, b = params
+        assert a.value == pytest.approx(1e6)
+        assert b.value == pytest.approx(3.0)
+        for param in params:
+            assert param.lineedit.text() == param.format % param.value
+            assert not param.slider.signalsBlocked()
+        assert a.slider.value() == a.steps - 1
+        assert _has_range_warning(a)
+        assert not _has_range_warning(b)
+        dialog.close()
+
+
+@pytest.mark.parametrize("text", ["nan", "inf", "-inf", "abc", ""])
+def test_line_edit_rejects_invalid_values(text: str):
+    """Typing a non-numeric or non-finite value restores the previous value."""
+    with qt_app_context(exec_loop=False):
+        param = _param(0.5)
+        param.lineedit.setText(text)
+        param.line_editing_finished()
+        assert param.value == 0.5
+        assert param.lineedit.text() == param.format % 0.5
+
+
+def test_line_edit_accepts_large_values():
+    """A large finite value is accepted, with the slider at its end."""
+    with qt_app_context(exec_loop=False):
+        param = _param(0.5)
+        param.lineedit.setText("1e300")
+        param.line_editing_finished()
+        assert param.value == 1e300
+        assert param.slider.value() == param.steps - 1
+        assert _has_range_warning(param)
+
+
+def test_steps_minimum():
+    """The parameter properties dialog requires at least two slider steps."""
+    assert not FitParamDataSet.steps.check_value(1)
+    assert FitParamDataSet.steps.check_value(2)

plotpy/widgets/fit.py

@@ -62,7 +62,7 @@
     restore_dataset,
     update_dataset,
 )
-from guidata.qthelpers import create_groupbox, exec_dialog
+from guidata.qthelpers import create_groupbox, exec_dialog, get_std_icon
 
 # Do not remove this import (used by optimization funcs)
 from numpy import inf  # noqa: F401
@@ -121,7 +121,7 @@ class FitParamDataSet(DataSet):
     value = FloatItem(_("Value"), default=0.0)
     min = FloatItem(_("Min"), default=-1.0)
     max = FloatItem(_("Max"), default=1.0).set_pos(col=1)
-    steps = IntItem(_("Steps"), default=5000)
+    steps = IntItem(_("Steps"), default=5000, min=2)
     format = StringItem(_("Format"), default="%.3f").set_pos(col=1)
     logscale = BoolItem(_("Logarithmic"), _("Scale"))
     unit = StringItem(_("Unit"), default="").set_pos(col=1)
@@ -169,6 +169,7 @@ def __init__(
         self.locked = locked
         self.prefix_label = None
         self.lineedit = None
+        self.range_warning = None
         self.unit_label = None
         self.slider = None
         self.button = None
@@ -216,6 +217,10 @@ def create_widgets(self, parent: QWidget, refresh_callback: Callable) -> None:
         self.button.clicked.connect(lambda: self.edit_param(parent))
         self.lineedit = QW.QLineEdit()
         self.lineedit.editingFinished.connect(self.line_editing_finished)
+        self.range_warning = self.lineedit.addAction(
+            get_std_icon("MessageBoxWarning"), QW.QLineEdit.TrailingPosition
+        )
+        self.range_warning.setVisible(False)
         self.unit_label = QW.QLabel(self.unit)
         self.slider = QW.QSlider()
         self.slider.setOrientation(QC.Qt.Horizontal)
@@ -278,14 +283,40 @@ def set_text(self, fmt: str = None) -> None:
             self.locked or (self.value == self.min and self.max == self.min)
         )
         self.lineedit.setDisabled(is_disabled)
+        self._update_range_warning()
+
+    def _update_range_warning(self) -> None:
+        """Show a warning when the value is outside the slider range"""
+        out_of_range = bool(
+            self.value is not None
+            and self.min is not None
+            and self.max is not None
+            and np.isfinite(self.value)
+            and not self.min <= self.value <= self.max
+        )
+        tooltip = ""
+        if out_of_range:
+            tooltip = _("This value is outside the slider range ({vmin} to {vmax})")
+            tooltip = tooltip.format(
+                vmin=self.format % self.min, vmax=self.format % self.max
+            )
+        self.range_warning.setVisible(out_of_range)
+        self.range_warning.setToolTip(tooltip)
+        self.lineedit.setToolTip(tooltip)
 
     def line_editing_finished(self):
         """Line editing finished"""
         try:
-            self.value = float(self.lineedit.text())
+            value = float(self.lineedit.text())
         except ValueError:
+            value = np.nan
+        if not np.isfinite(value):
+            # Not a valid value (including "nan" and "inf"): restore the previous one
             self.set_text()
+            return
+        self.value = value
         self.update_slider_value()
+        self._update_range_warning()
         self._refresh_callback()
 
     def slider_value_changed(self, int_value: int) -> None:
@@ -305,6 +336,29 @@ def slider_value_changed(self, int_value: int) -> None:
         self.set_text()
         self._refresh_callback()
 
+    def _get_slider_position(self) -> int | None:
+        """Return the slider position matching the value
+
+        Values outside the [min, max] range give the position of the nearest end of
+        the slider. This is the inverse of the mapping of `slider_value_changed`.
+
+        Returns:
+            Slider position, or None if the value is not finite or the range is empty
+        """
+        vmin, vmax, value = self.min, self.max, self.value
+        width = vmax - vmin
+        if not (np.isfinite(value) and np.isfinite(width) and width > 0):
+            return None
+        if value <= vmin:
+            ratio = 0.0
+        elif value >= vmax:
+            ratio = 1.0
+        elif self.logscale:
+            ratio = np.log10(1 + value - vmin) / np.log10(1 + width)
+        else:
+            ratio = (value - vmin) / width
+        return int(round(float(ratio) * (self.steps - 1)))
+
     def update_slider_value(self):
         """Update slider value"""
         if self.value is None or self.min is None or self.max is None:
@@ -317,16 +371,13 @@ def update_slider_value(self):
             self.slider.setEnabled(not self.locked)
             if self.slider.parentWidget() and self.slider.parentWidget().isVisible():
                 self.slider.show()
-            if self.logscale:
-                value_delta = max([np.log10(1 + self.value - self.min), 0.0])
-                total_delta = np.log10(1 + self.max - self.min)
-            else:
-                value_delta = self.value - self.min
-                total_delta = self.max - self.min
-            intval = int(self.steps * value_delta / total_delta)
-            self.slider.blockSignals(True)
-            self.slider.setValue(intval)
-            self.slider.blockSignals(False)
+            position = self._get_slider_position()
+            if position is not None:
+                was_blocked = self.slider.blockSignals(True)
+                try:
+                    self.slider.setValue(position)
+                finally:
+                    self.slider.blockSignals(was_blocked)
 
     def edit_param(self, parent: QWidget) -> None:
         """Edit param