From cc2603988b3751c6ded3f21ac09ec12c6a18e033 Mon Sep 17 00:00:00 2001 From: Thomas MALLET Date: Thu, 15 Oct 2026 14:40:12 +0200 Subject: [PATCH 1/2] Accept None in check_value for optional numeric items * [UPDATE] : guidata/dataset/dataitems.py: - NumericTypeItem.check_value accepts None when allow_none=True - IntItem.check_value skips the parity check for None * [ADD] : guidata/tests/unit/test_optional_numeric_check.py: - test optional int/float items accept None (empty field) - test non-None values keep min/even constraints - test required items keep rejecting None - test DataSetEditLayout accepts empty optional numeric fields * [UPDATE] : doc/release_notes/release_3.16.md: - add bug fix entry for empty optional numeric fields Assisted by : Opus 5.5 --- doc/release_notes/release_3.16.md | 2 + guidata/dataset/dataitems.py | 5 ++ .../tests/unit/test_optional_numeric_check.py | 82 +++++++++++++++++++ 3 files changed, 89 insertions(+) create mode 100644 guidata/tests/unit/test_optional_numeric_check.py diff --git a/doc/release_notes/release_3.16.md b/doc/release_notes/release_3.16.md index 3547de1..4f4dedb 100644 --- a/doc/release_notes/release_3.16.md +++ b/doc/release_notes/release_3.16.md @@ -9,3 +9,5 @@ * **`HistogramRangeItem` — histogram-backed interval selection** — added a new `DataItem` that edits two distinct `FloatItem` fields of the same dataset through a single histogram-backed range editor widget. The caller supplies a rendering payload (bin `counts`, a finite ordered `domain`, optional `y_max`, `minimum_width`, `auto_range` and `reset_range` proposals); guidata never inspects source arrays or computes a range on its own. The default `"range"` presentation shows a plain interval selector suitable for generic measurements (durations, thresholds, …); setting `display.presentation` to `"brightness_contrast"` adds brightness/contrast sliders and a linear transfer function overlay for image-oriented use cases. The item is transient: only the two linked numeric fields are persisted (HDF5/JSON/INI), so applications must restore the histogram context after loading before editing is possible. A `display.callback(instance, item, value)` is invoked once per valid edited pair, with both bounds already applied to the working dataset, matching the behavior of other guidata live callbacks. The JSON Schema exporter emits a dedicated `"histogram_range"` kind with `x-guidata-minimum-field`, `x-guidata-maximum-field` and `x-guidata-histogram-presentation` extension keywords so portable renderers can reproduce the same behavior. See the [examples](../examples.rst) and [`HistogramRangeItem` reference](../reference/dataset/dataitems.rst) documentation for a complete, tested duration-selection walkthrough. 🛠 Bug fixes: + +* **Empty optional numeric fields rejected in forms** — `IntItem` and `FloatItem` declared with `allow_none=True` accepted `None` when set programmatically, but `check_value` rejected it, so an empty optional field was highlighted as invalid and `DataSetEditLayout.check_all_values()` refused the whole form (e.g. on dialog validation). `check_value` now accepts `None` for optional numeric items, while still validating non-`None` values against their constraints (`min`, `max`, `nonzero`, `even`) and rejecting `None` for required items. diff --git a/guidata/dataset/dataitems.py b/guidata/dataset/dataitems.py index 12c7c22..bae9d1c 100644 --- a/guidata/dataset/dataitems.py +++ b/guidata/dataset/dataitems.py @@ -284,6 +284,8 @@ def check_value(self, value: float | int, raise_exception: bool = False) -> bool """Override DataItem method""" if not self.get_prop("data", "check_value", True): return True + if value is None and self.get_prop("data", "allow_none", False): + return True if not isinstance(value, self.type): if raise_exception: raise TypeError(f"Expected {self.type}, got {type(value)}") @@ -475,6 +477,9 @@ def check_value(self, value: int, raise_exception: bool = False) -> bool: valid = super().check_value(value, raise_exception=raise_exception) if not valid: return False + if value is None: + # Only reached for optional items (``allow_none=True``) + return True even = self.get_prop("data", "even") if even is not None: is_even = value // 2 == value / 2.0 diff --git a/guidata/tests/unit/test_optional_numeric_check.py b/guidata/tests/unit/test_optional_numeric_check.py new file mode 100644 index 0000000..2078531 --- /dev/null +++ b/guidata/tests/unit/test_optional_numeric_check.py @@ -0,0 +1,82 @@ +# -*- coding: utf-8 -*- +# +# Licensed under the terms of the BSD 3-Clause +# (see guidata/LICENSE for details) + +""" +Test check_value of optional numeric items + +Form widgets validate user input with ``check_value``: an empty field of an +optional numeric item (``allow_none=True``) is converted to None and must be +accepted, while required items keep rejecting None. +""" + +import pytest +from qtpy import QtWidgets as QW + +from guidata.dataset import DataSet +from guidata.dataset.dataitems import FloatItem, IntItem +from guidata.dataset.datatypes import DataItemVariable +from guidata.dataset.qtwidgets import DataSetEditLayout +from guidata.qthelpers import qt_app_context + + +class _OptionalNumbersDataSet(DataSet): + """Dataset with optional and required numeric items""" + + optional_int = IntItem("Optional int", default=None, min=1, allow_none=True) + optional_even = IntItem("Optional even", default=None, even=True, allow_none=True) + optional_float = FloatItem("Optional float", default=None, allow_none=True) + required_int = IntItem("Required int", default=1, min=1) + + +def _get_item(dataset: DataSet, name: str): + """Return the data item named ``name``""" + return next(item for item in dataset.get_items() if item.get_name() == name) + + +@pytest.mark.parametrize("name", ["optional_int", "optional_even", "optional_float"]) +def test_optional_numeric_item_accepts_none(name: str) -> None: + """Optional numeric items accept None, e.g. from an empty field""" + dataset = _OptionalNumbersDataSet() + variable = DataItemVariable(_get_item(dataset, name), dataset) + + assert variable.from_string("") is None + assert variable.check_value(None) is True + assert variable.check_value(None, raise_exception=True) is True + + +def test_optional_numeric_item_keeps_value_constraints() -> None: + """Optional numeric items still validate non-None values""" + dataset = _OptionalNumbersDataSet() + + assert _get_item(dataset, "optional_int").check_value(0) is False + assert _get_item(dataset, "optional_int").check_value(2) is True + assert _get_item(dataset, "optional_even").check_value(3) is False + assert _get_item(dataset, "optional_even").check_value(4) is True + with pytest.raises(ValueError): + _get_item(dataset, "optional_int").check_value(0, raise_exception=True) + + +def test_required_numeric_item_rejects_none() -> None: + """Required numeric items keep rejecting None""" + item = _get_item(_OptionalNumbersDataSet(), "required_int") + + assert item.check_value(None) is False + with pytest.raises(TypeError): + item.check_value(None, raise_exception=True) + + +def test_empty_optional_numeric_fields_are_valid_in_form() -> None: + """A form with empty optional numeric fields passes validation""" + with qt_app_context(): + dialog = QW.QDialog() + dataset = _OptionalNumbersDataSet() + layout = DataSetEditLayout(dialog, dataset, QW.QGridLayout(dialog)) + try: + assert layout.check_all_values() is True + layout.accept_changes() + assert dataset.optional_int is None + assert dataset.optional_float is None + finally: + dialog.deleteLater() From 0d4bde7aa74efe116dd734ae582f490bb245730b Mon Sep 17 00:00:00 2001 From: Thomas MALLET Date: Thu, 15 Oct 2026 14:56:12 +0200 Subject: [PATCH 2/2] Reject malformed input in optional numeric text fields * [UPDATE] : guidata/dataset/qtitemwidgets.py: - add LineEditWidget.parse_text distinguishing empty from malformed text - use parse_text in check and line_edit_changed * [UPDATE] : guidata/tests/unit/test_optional_numeric_check.py: - add test_malformed_optional_numeric_input_is_invalid - add test_display_callback_ignores_malformed_optional_numeric_input * [UPDATE] : doc/release_notes/release_3.16.md: - state that malformed entries remain invalid --- doc/release_notes/release_3.16.md | 2 +- guidata/dataset/qtitemwidgets.py | 29 ++++++++-- .../tests/unit/test_optional_numeric_check.py | 56 +++++++++++++++++++ 3 files changed, 82 insertions(+), 5 deletions(-) diff --git a/doc/release_notes/release_3.16.md b/doc/release_notes/release_3.16.md index 4f4dedb..86dac3e 100644 --- a/doc/release_notes/release_3.16.md +++ b/doc/release_notes/release_3.16.md @@ -10,4 +10,4 @@ 🛠 Bug fixes: -* **Empty optional numeric fields rejected in forms** — `IntItem` and `FloatItem` declared with `allow_none=True` accepted `None` when set programmatically, but `check_value` rejected it, so an empty optional field was highlighted as invalid and `DataSetEditLayout.check_all_values()` refused the whole form (e.g. on dialog validation). `check_value` now accepts `None` for optional numeric items, while still validating non-`None` values against their constraints (`min`, `max`, `nonzero`, `even`) and rejecting `None` for required items. +* **Empty optional numeric fields rejected in forms** — `IntItem` and `FloatItem` declared with `allow_none=True` accepted `None` when set programmatically, but `check_value` rejected it, so an empty optional field was highlighted as invalid and `DataSetEditLayout.check_all_values()` refused the whole form (e.g. on dialog validation). `check_value` now accepts `None` for optional numeric items, while still validating non-`None` values against their constraints (`min`, `max`, `nonzero`, `even`) and rejecting `None` for required items. Text fields only accept `None` from an empty input: malformed entries (e.g. `abc` in a numeric field) remain invalid instead of being stored as `None`. diff --git a/guidata/dataset/qtitemwidgets.py b/guidata/dataset/qtitemwidgets.py index ed14375..4114df0 100644 --- a/guidata/dataset/qtitemwidgets.py +++ b/guidata/dataset/qtitemwidgets.py @@ -529,10 +529,31 @@ def get(self) -> None: # state changed from empty string to None self.line_edit_changed(value) + def parse_text(self, text: str | None) -> tuple[Any, bool]: + """Convert widget text into an item value and validate it + + Non-empty text converted to None could not be parsed (e.g. "abc" for a + numeric item): it is invalid, even for optional items, which only accept + None from an empty field. + + Args: + text: Widget text, or None for a None item value + + Returns: + Tuple of the item value and its validity + """ + if text is None: + value = None + else: + value = self.item.from_string(str(text)) + if value is None and str(text).strip(): + return value, False + return value, self.item.check_value(value) + def line_edit_changed(self, qvalue: str | None) -> None: """QLineEdit validator""" - value = self.item.from_string(str(qvalue)) if qvalue is not None else None - if not self.item.check_value(value): + value, valid = self.parse_text(qvalue) + if not valid: self.edit.setStyleSheet("background-color:rgb(255, 175, 90);") else: self.edit.setStyleSheet(_get_readonly_stylesheet()) @@ -541,6 +562,7 @@ def line_edit_changed(self, qvalue: str | None) -> None: self.notify_value_change() def update(self, value: Any) -> None: + """Update the display callback with the given value""" cb = self.item.get_prop_value("display", "value_callback", None) if cb is not None: cb(value) @@ -559,8 +581,7 @@ def check(self) -> bool: Returns: True if item value is valid """ - value = self.item.from_string(str(self.edit.text())) - return self.item.check_value(value) + return self.parse_text(str(self.edit.text()))[1] def set_state(self): """Update the visual status of the widget and modify the widget to readonly if diff --git a/guidata/tests/unit/test_optional_numeric_check.py b/guidata/tests/unit/test_optional_numeric_check.py index 2078531..a504149 100644 --- a/guidata/tests/unit/test_optional_numeric_check.py +++ b/guidata/tests/unit/test_optional_numeric_check.py @@ -80,3 +80,59 @@ def test_empty_optional_numeric_fields_are_valid_in_form() -> None: assert dataset.optional_float is None finally: dialog.deleteLater() + + +def _get_widget(layout: DataSetEditLayout, name: str): + """Return the terminal widget of the data item named ``name``""" + return next( + widget + for widget in layout.get_terminal_widgets() + if widget.item.item.get_name() == name + ) + + +@pytest.mark.parametrize("name", ["optional_int", "optional_float"]) +@pytest.mark.parametrize("text", ["abc", "1/0", "1e"]) +def test_malformed_optional_numeric_input_is_invalid(name: str, text: str) -> None: + """Malformed nonempty input in an optional numeric field blocks the form""" + with qt_app_context(): + dialog = QW.QDialog() + dataset = _OptionalNumbersDataSet() + layout = DataSetEditLayout(dialog, dataset, QW.QGridLayout(dialog)) + try: + widget = _get_widget(layout, name) + widget.edit.setText(text) + assert widget.check() is False + assert layout.check_all_values() is False + widget.edit.setText(" ") + assert widget.check() is True + assert layout.check_all_values() is True + finally: + dialog.deleteLater() + + +def test_display_callback_ignores_malformed_optional_numeric_input() -> None: + """Only empty or valid input triggers the display callback""" + values = [] + + class _CallbackDataSet(DataSet): + """Dataset with an optional numeric item having a display callback""" + + value = IntItem("Value", default=None, allow_none=True).set_prop( + "display", callback=lambda _instance, _item, value: values.append(value) + ) + + with qt_app_context(): + dialog = QW.QDialog() + dataset = _CallbackDataSet() + layout = DataSetEditLayout(dialog, dataset, QW.QGridLayout(dialog)) + try: + widget = _get_widget(layout, "value") + values.clear() + widget.edit.setText("abc") + assert not values + widget.edit.setText("3") + widget.edit.setText("") + assert values == [3, None] + finally: + dialog.deleteLater()