Skip to content

Commit 12c9f5d

Browse files
Merge pull request #77 from PlotPyStack/hotfix/fit-slider-overflow
Fix OverflowError in the curve fitting dialog when a fitted value is outside the slider range
2 parents 300efd3 + 861770c commit 12c9f5d

5 files changed

Lines changed: 219 additions & 14 deletions

File tree

‎.github/workflows/test_pyqt5.yml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ jobs:
1818
env:
1919
DISPLAY: ':99.0'
2020

21-
runs-on: ubuntu-latest
21+
# Python 3.9 is not available on newer Ubuntu runners (ubuntu-latest)
22+
runs-on: ubuntu-24.04
2223
strategy:
2324
fail-fast: false
2425
matrix:

‎doc/release_notes/release_2.11.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Version 2.11 #
22

3+
## PlotPy Version 2.11.2 ##
4+
5+
🛠️ Bug fixes:
6+
7+
* **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))
8+
39
## PlotPy Version 2.11.1 ##
410

511
🛠️ Bug fixes:

‎plotpy/locale/fr/LC_MESSAGES/plotpy.po‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1628,6 +1628,10 @@ msgstr "Paramètre d'ajustement"
16281628
msgid "Edit '{name}' fit parameter properties"
16291629
msgstr "Modifier les propriétés du paramètre d'ajustement '{name}'"
16301630

1631+
#, python-brace-format
1632+
msgid "This value is outside the slider range ({vmin} to {vmax})"
1633+
msgstr "Cette valeur est en dehors de la plage du curseur ({vmin} à {vmax})"
1634+
16311635
msgid "Fit parameters"
16321636
msgstr "Paramètres d'ajustement"
16331637

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
# -*- coding: utf-8 -*-
2+
#
3+
# Licensed under the terms of the BSD 3-Clause
4+
# (see plotpy/LICENSE for details)
5+
6+
"""Curve fitting parameter sliders: values outside the slider range, non-finite
7+
values and invalid inputs keep the fit dialog usable."""
8+
9+
from __future__ import annotations
10+
11+
import numpy as np
12+
import pytest
13+
from guidata.qthelpers import qt_app_context
14+
15+
from plotpy.widgets.fit import FitDialog, FitParam, FitParamDataSet
16+
17+
18+
def _param(
19+
value: float = 0.5, vmin: float = 0.0, vmax: float = 1.0, logscale: bool = False
20+
) -> FitParam:
21+
"""Return a fit parameter with its widgets, updated for the given value"""
22+
param = FitParam("p", 0.5, 0.0, 1.0, logscale=logscale)
23+
param.create_widgets(None, lambda: None)
24+
param.min, param.max, param.value = vmin, vmax, value
25+
param.update()
26+
return param
27+
28+
29+
def _has_range_warning(param: FitParam) -> bool:
30+
"""Return True if the parameter shows the out-of-range warning"""
31+
return param.range_warning.isVisible() and bool(param.lineedit.toolTip())
32+
33+
34+
@pytest.mark.parametrize("logscale", [False, True])
35+
@pytest.mark.parametrize("value", [1e15, 2.0, -1.0, -1e15])
36+
def test_value_outside_range_clamps_slider(value: float, logscale: bool):
37+
"""The slider stays at the nearest end and a warning is shown."""
38+
with qt_app_context(exec_loop=False):
39+
param = _param(value, logscale=logscale)
40+
expected = param.steps - 1 if value > param.max else 0
41+
assert param.slider.value() == expected
42+
assert not param.slider.signalsBlocked()
43+
assert param.value == value
44+
assert _has_range_warning(param)
45+
param.value = 0.5
46+
param.update()
47+
assert not _has_range_warning(param)
48+
49+
50+
@pytest.mark.parametrize(
51+
"value, vmin, vmax",
52+
[
53+
(np.nan, 0.0, 1.0),
54+
(np.inf, 0.0, 1.0),
55+
(-np.inf, 0.0, 1.0),
56+
(2.0, 1.0, 1.0),
57+
(np.float64(2.0), 1.0, 1.0),
58+
(1e308, -1e308, 1e308),
59+
],
60+
)
61+
def test_undefined_slider_position(value: float, vmin: float, vmax: float):
62+
"""Non-finite values and empty ranges leave the slider position unchanged."""
63+
with qt_app_context(exec_loop=False):
64+
param = _param(0.5)
65+
position = param.slider.value()
66+
param.min, param.max, param.value = vmin, vmax, value
67+
param.update()
68+
assert param.slider.value() == position
69+
assert not param.slider.signalsBlocked()
70+
71+
72+
@pytest.mark.parametrize("logscale", [False, True])
73+
def test_slider_mapping_round_trip(logscale: bool):
74+
"""Slider position -> value -> slider position gives the same position."""
75+
with qt_app_context(exec_loop=False):
76+
param = _param(0.5, 0.0, 10.0, logscale)
77+
last = param.steps - 1
78+
for position in (0, 1, 1234, last - 1, last):
79+
param.slider_value_changed(position)
80+
param.update_slider_value()
81+
assert param.slider.value() == position
82+
param.value = param.min
83+
param.update_slider_value()
84+
assert param.slider.value() == 0
85+
param.value = param.max
86+
param.update_slider_value()
87+
assert param.slider.value() == last
88+
89+
90+
def test_autofit_with_value_far_outside_range():
91+
"""All parameters are updated when the fit leaves a slider range."""
92+
x = np.linspace(0.0, 1.0, 50)
93+
y = 1e6 * x + 3.0
94+
95+
def fitfunc(x: np.ndarray, params: list[float]) -> np.ndarray:
96+
a, b = params
97+
return a * x + b
98+
99+
with qt_app_context(exec_loop=False):
100+
# The fitted slope is 1e6 times the slider range width: its slider position
101+
# used to exceed the 32-bit integer range accepted by `QSlider.setValue`
102+
params = [FitParam("a", 0.5, 0.0, 1.0), FitParam("b", 0.0, -5.0, 5.0)]
103+
dialog = FitDialog(edit=True, auto_fit=True)
104+
dialog.set_data(x, y, fitfunc, params)
105+
dialog.autofit()
106+
a, b = params
107+
assert a.value == pytest.approx(1e6)
108+
assert b.value == pytest.approx(3.0)
109+
for param in params:
110+
assert param.lineedit.text() == param.format % param.value
111+
assert not param.slider.signalsBlocked()
112+
assert a.slider.value() == a.steps - 1
113+
assert _has_range_warning(a)
114+
assert not _has_range_warning(b)
115+
dialog.close()
116+
117+
118+
@pytest.mark.parametrize("text", ["nan", "inf", "-inf", "abc", ""])
119+
def test_line_edit_rejects_invalid_values(text: str):
120+
"""Typing a non-numeric or non-finite value restores the previous value."""
121+
with qt_app_context(exec_loop=False):
122+
param = _param(0.5)
123+
param.lineedit.setText(text)
124+
param.line_editing_finished()
125+
assert param.value == 0.5
126+
assert param.lineedit.text() == param.format % 0.5
127+
128+
129+
def test_line_edit_accepts_large_values():
130+
"""A large finite value is accepted, with the slider at its end."""
131+
with qt_app_context(exec_loop=False):
132+
param = _param(0.5)
133+
param.lineedit.setText("1e300")
134+
param.line_editing_finished()
135+
assert param.value == 1e300
136+
assert param.slider.value() == param.steps - 1
137+
assert _has_range_warning(param)
138+
139+
140+
def test_steps_minimum():
141+
"""The parameter properties dialog requires at least two slider steps."""
142+
assert not FitParamDataSet.steps.check_value(1)
143+
assert FitParamDataSet.steps.check_value(2)

‎plotpy/widgets/fit.py‎

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
restore_dataset,
6363
update_dataset,
6464
)
65-
from guidata.qthelpers import create_groupbox, exec_dialog
65+
from guidata.qthelpers import create_groupbox, exec_dialog, get_std_icon
6666

6767
# Do not remove this import (used by optimization funcs)
6868
from numpy import inf # noqa: F401
@@ -121,7 +121,7 @@ class FitParamDataSet(DataSet):
121121
value = FloatItem(_("Value"), default=0.0)
122122
min = FloatItem(_("Min"), default=-1.0)
123123
max = FloatItem(_("Max"), default=1.0).set_pos(col=1)
124-
steps = IntItem(_("Steps"), default=5000)
124+
steps = IntItem(_("Steps"), default=5000, min=2)
125125
format = StringItem(_("Format"), default="%.3f").set_pos(col=1)
126126
logscale = BoolItem(_("Logarithmic"), _("Scale"))
127127
unit = StringItem(_("Unit"), default="").set_pos(col=1)
@@ -169,6 +169,7 @@ def __init__(
169169
self.locked = locked
170170
self.prefix_label = None
171171
self.lineedit = None
172+
self.range_warning = None
172173
self.unit_label = None
173174
self.slider = None
174175
self.button = None
@@ -216,6 +217,10 @@ def create_widgets(self, parent: QWidget, refresh_callback: Callable) -> None:
216217
self.button.clicked.connect(lambda: self.edit_param(parent))
217218
self.lineedit = QW.QLineEdit()
218219
self.lineedit.editingFinished.connect(self.line_editing_finished)
220+
self.range_warning = self.lineedit.addAction(
221+
get_std_icon("MessageBoxWarning"), QW.QLineEdit.TrailingPosition
222+
)
223+
self.range_warning.setVisible(False)
219224
self.unit_label = QW.QLabel(self.unit)
220225
self.slider = QW.QSlider()
221226
self.slider.setOrientation(QC.Qt.Horizontal)
@@ -278,14 +283,40 @@ def set_text(self, fmt: str = None) -> None:
278283
self.locked or (self.value == self.min and self.max == self.min)
279284
)
280285
self.lineedit.setDisabled(is_disabled)
286+
self._update_range_warning()
287+
288+
def _update_range_warning(self) -> None:
289+
"""Show a warning when the value is outside the slider range"""
290+
out_of_range = bool(
291+
self.value is not None
292+
and self.min is not None
293+
and self.max is not None
294+
and np.isfinite(self.value)
295+
and not self.min <= self.value <= self.max
296+
)
297+
tooltip = ""
298+
if out_of_range:
299+
tooltip = _("This value is outside the slider range ({vmin} to {vmax})")
300+
tooltip = tooltip.format(
301+
vmin=self.format % self.min, vmax=self.format % self.max
302+
)
303+
self.range_warning.setVisible(out_of_range)
304+
self.range_warning.setToolTip(tooltip)
305+
self.lineedit.setToolTip(tooltip)
281306

282307
def line_editing_finished(self):
283308
"""Line editing finished"""
284309
try:
285-
self.value = float(self.lineedit.text())
310+
value = float(self.lineedit.text())
286311
except ValueError:
312+
value = np.nan
313+
if not np.isfinite(value):
314+
# Not a valid value (including "nan" and "inf"): restore the previous one
287315
self.set_text()
316+
return
317+
self.value = value
288318
self.update_slider_value()
319+
self._update_range_warning()
289320
self._refresh_callback()
290321

291322
def slider_value_changed(self, int_value: int) -> None:
@@ -305,6 +336,29 @@ def slider_value_changed(self, int_value: int) -> None:
305336
self.set_text()
306337
self._refresh_callback()
307338

339+
def _get_slider_position(self) -> int | None:
340+
"""Return the slider position matching the value
341+
342+
Values outside the [min, max] range give the position of the nearest end of
343+
the slider. This is the inverse of the mapping of `slider_value_changed`.
344+
345+
Returns:
346+
Slider position, or None if the value is not finite or the range is empty
347+
"""
348+
vmin, vmax, value = self.min, self.max, self.value
349+
width = vmax - vmin
350+
if not (np.isfinite(value) and np.isfinite(width) and width > 0):
351+
return None
352+
if value <= vmin:
353+
ratio = 0.0
354+
elif value >= vmax:
355+
ratio = 1.0
356+
elif self.logscale:
357+
ratio = np.log10(1 + value - vmin) / np.log10(1 + width)
358+
else:
359+
ratio = (value - vmin) / width
360+
return int(round(float(ratio) * (self.steps - 1)))
361+
308362
def update_slider_value(self):
309363
"""Update slider value"""
310364
if self.value is None or self.min is None or self.max is None:
@@ -317,16 +371,13 @@ def update_slider_value(self):
317371
self.slider.setEnabled(not self.locked)
318372
if self.slider.parentWidget() and self.slider.parentWidget().isVisible():
319373
self.slider.show()
320-
if self.logscale:
321-
value_delta = max([np.log10(1 + self.value - self.min), 0.0])
322-
total_delta = np.log10(1 + self.max - self.min)
323-
else:
324-
value_delta = self.value - self.min
325-
total_delta = self.max - self.min
326-
intval = int(self.steps * value_delta / total_delta)
327-
self.slider.blockSignals(True)
328-
self.slider.setValue(intval)
329-
self.slider.blockSignals(False)
374+
position = self._get_slider_position()
375+
if position is not None:
376+
was_blocked = self.slider.blockSignals(True)
377+
try:
378+
self.slider.setValue(position)
379+
finally:
380+
self.slider.blockSignals(was_blocked)
330381

331382
def edit_param(self, parent: QWidget) -> None:
332383
"""Edit param

0 commit comments

Comments
 (0)