From d10e99b0527215ce3e90c523169f3f1553dd7b46 Mon Sep 17 00:00:00 2001 From: Daniel Mulholland Date: Fri, 6 Nov 2026 21:59:00 +1000 Subject: [PATCH 1/6] Filter row for large results tables and the Data Tables A per-column filter strip in the table's own header (Ctrl+F or the header menu), for tables of more than 10 rows. Expressions: text, wildcards, quoted phrases, !, =, >, >=, <, <=, a..b ranges, blank/nonblank, space = AND, | or , = OR. Typing is debounced (150 ms); a pass hides rows with setRowHidden and copies no data. A 'Showing N of M - Clear' note shows while a filter is active. Results tables (via _make_table) and the analytic per-element table use the cell text and exact raw values. The Data Tables filter the model objects the rows were filled from, so hidden rows never build their cell widgets, and CSV export reads only the visible rows (and says so). An edited row that stops matching stays shown until the next time the user types. --- raptor/gui/table_filter.py | 229 +++++++++++++ raptor/gui/ui/analytic_dialog.py | 7 + raptor/gui/ui/data_tables.py | 115 ++++++- raptor/gui/ui/results.py | 4 + raptor/gui/ui/table_filter_bar.py | 415 ++++++++++++++++++++++++ tests/unit/test_gui_table_filter_bar.py | 310 ++++++++++++++++++ tests/unit/test_table_filter.py | 90 +++++ 7 files changed, 1166 insertions(+), 4 deletions(-) create mode 100644 raptor/gui/table_filter.py create mode 100644 raptor/gui/ui/table_filter_bar.py create mode 100644 tests/unit/test_gui_table_filter_bar.py create mode 100644 tests/unit/test_table_filter.py diff --git a/raptor/gui/table_filter.py b/raptor/gui/table_filter.py new file mode 100644 index 00000000..e76d0c5c --- /dev/null +++ b/raptor/gui/table_filter.py @@ -0,0 +1,229 @@ +"""Per-column filter expressions for the GUI's big tables (pure Python, no Qt). + +A filter box holds one small expression. ``compile_filter`` turns it into a +predicate over a cell, given as its display text and, when the table has +one, its raw number. The syntax, kept deliberately small: + + pump text contains "pump" (case-insensitive) + "main pump" a phrase, spaces included + Pump* *_B P?mp wildcards (* any run, ? one character); with a + wildcard the whole cell must match, so ``Pump*`` means + "starts with Pump" and ``*pump*`` means "contains" + =Pump_A the whole cell equals the text (also ``=5`` numerically) + !pump NOT: cells that do not match the rest of the term + >0.9 <=100 >=5 <2 =3 !=0 number comparisons + 5..10 a number from 5 to 10 inclusive + blank nonblank empty / non-empty cell + +Terms separated by spaces must all match (AND); ``|`` or ``,`` between +terms means OR, and binds loosest: ``>5 <10 | blank`` is "(>5 and <10) or +blank". A number term compares the cell's raw value when there is one, +otherwise the number read from its text (commas, ``%``, a ``>`` censor mark +and a trailing unit are ignored); a cell with no number never matches one. + +A bad expression never raises: the result has ``valid=False`` and a message, +and matches everything, so the table is left unfiltered while the box shows +the problem. +""" +from __future__ import annotations + +import re +from collections.abc import Callable +from dataclasses import dataclass + +#: Predicate over (display text, raw number or None). +CellTest = Callable[[str, "float | None"], bool] + +SYNTAX_HELP = ( + "Type to filter this column (case-insensitive).\n" + " pump contains 'pump' Pump* starts with 'Pump'\n" + " >0.9 <=100 number comparisons 5..10 a number from 5 to 10\n" + " =Pump_A the whole cell equals !pump does not contain 'pump'\n" + " blank / nonblank \"two words\" a phrase\n" + "Space = AND, | or , = OR. Example: >5 <10 | blank") + +_NUM = re.compile(r"[-+]?(?:\d[\d,]*\.?\d*|\.\d+)(?:[eE][-+]?\d+)?") +_UNIT_TAIL = re.compile(r"^\s*(?:%|[^\W\d_][^\d]{0,11})?\s*$") +_OPS = (">=", "<=", "!=", ">", "<", "=") + + +class FilterSyntaxError(ValueError): + """A bad expression; the message is shown to the user.""" + + +@dataclass(frozen=True) +class CompiledFilter: + test: CellTest + valid: bool = True + empty: bool = False + message: str = "" + + def __call__(self, text: str, raw: float | None = None) -> bool: + return self.test(text, raw) + + +def parse_number(text: str) -> float | None: + """The number a cell shows: ``1,234.5``, ``96.2%``, ``>1000``, ``3 hours`` + read as numbers; anything else None.""" + s = text.strip().lstrip("<>≥≤~≈").strip() + m = _NUM.match(s) + if m is None: + return None + if not _UNIT_TAIL.match(s[m.end():]): + return None + try: + return float(m.group().replace(",", "")) + except ValueError: + return None + + +def _cell_number(text: str, raw: float | None) -> float | None: + return raw if raw is not None else parse_number(text) + + +def _split_or(expr: str) -> list[str]: + """Split on | and , outside double quotes.""" + parts, cur, quoted = [], [], False + for ch in expr: + if ch == '"': + quoted = not quoted + if ch in "|," and not quoted: + parts.append("".join(cur)) + cur = [] + else: + cur.append(ch) + if quoted: + raise FilterSyntaxError('Unclosed quote ("') + parts.append("".join(cur)) + return parts + + +def _split_and(group: str) -> list[str]: + """Split on whitespace outside double quotes; quotes stay on the term.""" + terms, cur, quoted = [], [], False + for ch in group: + if ch == '"': + quoted = not quoted + cur.append(ch) + elif ch.isspace() and not quoted: + if cur: + terms.append("".join(cur)) + cur = [] + else: + cur.append(ch) + if cur: + terms.append("".join(cur)) + return terms + + +def _unquote(s: str) -> str: + return s[1:-1] if len(s) >= 2 and s[0] == s[-1] == '"' else s.replace('"', "") + + +def _glob(pattern: str) -> re.Pattern[str]: + out = "".join(".*" if c == "*" else "." if c == "?" else re.escape(c) + for c in pattern) + return re.compile(f"^{out}$", re.IGNORECASE | re.DOTALL) + + +def _operand(text: str, op: str) -> float: + n = parse_number(text) + if n is None: + raise FilterSyntaxError(f"'{op}' needs a number, not '{text}'") + return n + + +def _term(term: str) -> CellTest: + low = term.lower() + if low == "blank" or low == "empty": + return lambda t, r: not t.strip() + if low == "nonblank": + return lambda t, r: bool(t.strip()) + if term.startswith("!") and not term.startswith("!="): + rest = term[1:] + if not rest: + raise FilterSyntaxError("'!' needs something after it") + inner = _term(rest) + return lambda t, r: not inner(t, r) + for op in _OPS: + if term.startswith(op): + return _comparison(op, term[len(op):]) + if ".." in term and not term.startswith('"'): + return _range_term(term) + return _text_term(term) + + +def _range_term(term: str) -> CellTest: + lo_s, _, hi_s = term.partition("..") + lo, hi = _operand(lo_s, ".."), _operand(hi_s, "..") + if lo > hi: + lo, hi = hi, lo + + def in_range(t: str, r: float | None) -> bool: + n = _cell_number(t, r) + return n is not None and lo <= n <= hi + return in_range + + +def _text_term(term: str) -> CellTest: + body = _unquote(term) + if not body: + raise FilterSyntaxError("Empty text") + if "*" in body or "?" in body: + rx = _glob(body) + return lambda t, r: rx.match(t) is not None + needle = body.lower() + return lambda t, r: needle in t.lower() + + +def _comparison(op: str, operand: str) -> CellTest: + operand = _unquote(operand.strip()) + if not operand and op not in ("=", "!="): + raise FilterSyntaxError(f"'{op}' needs a number after it") + if op in ("=", "!="): + number = parse_number(operand) + want = operand.lower() + rx = _glob(operand) if ("*" in operand or "?" in operand) else None + + def equal(t: str, r: float | None) -> bool: + if number is not None: + n = _cell_number(t, r) + if n is not None: + return n == number + if rx is not None: + return rx.match(t) is not None + return t.strip().lower() == want + if op == "=": + return equal + return lambda t, r: not equal(t, r) + ref = _operand(operand, op) + compare = {">": lambda a: a > ref, "<": lambda a: a < ref, + ">=": lambda a: a >= ref, "<=": lambda a: a <= ref}[op] + + def numeric(t: str, r: float | None) -> bool: + n = _cell_number(t, r) + return n is not None and compare(n) + return numeric + + +def compile_filter(expr: str) -> CompiledFilter: + """The predicate for a filter box's text (see the module docstring).""" + if not expr.strip(): + return CompiledFilter(lambda t, r: True, empty=True) + try: + groups = [] + for part in _split_or(expr): + terms = [_term(t) for t in _split_and(part)] + if terms: + groups.append(terms) + if not groups: + return CompiledFilter(lambda t, r: True, empty=True) + except FilterSyntaxError as exc: + return CompiledFilter(lambda t, r: True, valid=False, message=str(exc)) + + if len(groups) == 1 and len(groups[0]) == 1: + return CompiledFilter(groups[0][0]) + + def test(t: str, r: float | None) -> bool: + return any(all(f(t, r) for f in g) for g in groups) + return CompiledFilter(test) diff --git a/raptor/gui/ui/analytic_dialog.py b/raptor/gui/ui/analytic_dialog.py index 2b821862..092f129e 100644 --- a/raptor/gui/ui/analytic_dialog.py +++ b/raptor/gui/ui/analytic_dialog.py @@ -42,6 +42,7 @@ from raptor.core.analytical.limits import max_bdd_nodes_allowed, memory_text from raptor.core.analytical.errors import AnalyticalError from raptor.core.analytical.network import MAX_VARIABLES +from raptor.gui.ui.table_filter_bar import FilterBar from raptor.gui.ui import msgbox from raptor.gui.resources.design_tokens import ( RADIUS, SPACING, TYPE, ThemeManager, active_colour) @@ -150,6 +151,9 @@ def __init__(self, value: float | None, digits: int = 6, *, Qt.AlignmentFlag.AlignRight | Qt.AlignmentFlag.AlignVCenter) if value is not None: self.setToolTip(_fmt_full(value)) + # The exact number, for the table's filter row (>0.99 compares + # this, not the rounded text). + self.setData(Qt.ItemDataRole.UserRole + 1, float(value)) def __lt__(self, other: QTableWidgetItem) -> bool: mine = self._value @@ -848,6 +852,9 @@ def _build_elements_table(self) -> QTableWidget: table.setHorizontalScrollMode( QAbstractItemView.ScrollMode.ScrollPerPixel) table.viewport().installEventFilter(self) + # Thousands of elements: a per-column filter row (shown on request, + # and only when there are more than a handful of rows). + FilterBar.attach(table, raw_role=Qt.ItemDataRole.UserRole + 1) return table def _wrap_with_phase_picker(self, tabs: QWidget) -> QWidget: diff --git a/raptor/gui/ui/data_tables.py b/raptor/gui/ui/data_tables.py index e7b0f959..82da8878 100644 --- a/raptor/gui/ui/data_tables.py +++ b/raptor/gui/ui/data_tables.py @@ -27,7 +27,8 @@ from raptor.gui.ui import filedialogs as fdg from raptor.gui.resources.design_tokens import ThemeManager, SPACING -from raptor.gui.ui.distro_combo import fill_distro_combo +from raptor.gui.ui.distro_combo import distro_display_name, fill_distro_combo +from raptor.gui.ui.table_filter_bar import FilterBar from raptor.gui.ui import param_entry from raptor.core.distro_rules import ( DISTRO_PARAM_LABELS, validate_distro_params, @@ -393,6 +394,7 @@ def __init__(self, parent=None): table.horizontalHeader().sortIndicatorChanged.connect( lambda *_a, t=table: QTimer.singleShot( 0, lambda t=t: self._realize_visible(t))) + self._attach_filters() self._tabs.currentChanged.connect(self._on_tab_changed) # Click a row -> jump to it on the canvas (entering its hierarchy @@ -694,6 +696,15 @@ def refresh(self, force: bool = True): self._events_table) else 0) item = current.item(row, key_col) keep_key = item.text() if item is not None else None + # An edit that makes this row stop matching a filter must not + # hide it under the cursor: keep it shown until the next time + # the user types in a filter box. + bar = getattr(self, "_filter_bars", {}).get(current) + if bar is not None and bar.is_active() and keep_key is not None: + try: + bar.keep_visible = {int(keep_key)} + except ValueError: + pass # Sorting OFF while rows are being inserted (an active sort # indicator + insertRow = scattered rows); back on after. _all_tables: list[QTableWidget] = [ @@ -912,8 +923,12 @@ def _export_current_table_csv(self) -> None: return tab_name = self._tabs.tabText(self._tabs.currentIndex()) default = tab_name.replace(" ", "_").lower() + ".csv" - path, _f = fdg.getSaveFileName( - self, "Export CSV", default, "CSV (*.csv)") + title = "Export CSV" + if self._filter_active(table): + shown, total = self._filter_bars[table].rows_shown_of_total() + title = (f"Export CSV: only the {shown:,} of {total:,} rows " + "the filter shows") + path, _f = fdg.getSaveFileName(self, title, default, "CSV (*.csv)") if not path: return @@ -971,6 +986,94 @@ def _apply_param_tips(self, t, row: int, distro: int, which: str): # Cell widgets on demand (#146) # ------------------------------------------------------------------ + # ------------------------------------------------------------------ + # Filter row (the grids with many rows) + # ------------------------------------------------------------------ + + #: Sparing-table columns a filter can read without the row's cell + #: widgets: Name, ID and Sparing Type (from the model). + _SPARING_FILTER_COLS = {0, 1, 2} + + def _attach_filters(self) -> None: + """A per-column filter row on the grids that can grow long. It reads + the model objects the rows were filled from, not the cell widgets, so + a row the filter hides never gets its widgets built.""" + self._filter_bars: dict[QTableWidget, FilterBar] = {} + for table, matcher, cols in ( + (self._blocks_table, self._block_matcher, None), + (self._nodes_table, None, None), + (self._events_table, None, None), + (self._sparing_table, self._sparing_matcher, + self._SPARING_FILTER_COLS), + (self._pm_table, None, None)): + bar = FilterBar.attach( + table, matcher=matcher, filter_columns=cols, + row_key=lambda r, t=table: self._row_key(t, r)) + bar.changed.connect( + lambda *_a, t=table: self._after_filter(t)) + self._filter_bars[table] = bar + + def _after_filter(self, t: QTableWidget) -> None: + """Rows came and went: build the widgets of the ones now in view.""" + if t in self._lazy_pending: + self._realize_visible(t) + + def _filter_active(self, t: QTableWidget) -> bool: + bar = getattr(self, "_filter_bars", {}).get(t) + return bar is not None and bar.is_active() + + def _block_cell(self, t: QTableWidget, row: int, col: int, + bid: int | None) -> tuple[str, float | None]: + """What the Blocks grid shows in a cell, from the model (the widget + columns) or the cell's item.""" + b = self._view.blocks.get(bid) if self._view is not None \ + and bid is not None else None + if b is not None and col in (2, 7): + code = b.fail_distro if col == 2 else b.repair_distro + return distro_display_name(code, is_failure=col == 2), None + if b is not None and col in (3, 4, 5, 8, 9, 10): + which = "F" if col < 7 else "R" + code = b.fail_distro if which == "F" else b.repair_distro + vals = b.fail_values if which == "F" else b.repair_values + i = col - 3 if which == "F" else col - 8 + shown = self._entry_values(code, list(vals)) + if i < len(shown): + return f"{shown[i]:.6g}", float(shown[i]) + return "", None + if b is not None and col == 12: + from raptor.gui.ui.units_conv import native_to_display + v = native_to_display( + b.pm_freq, getattr(self, "_native_units", "") or "", + getattr(self, "_display_unit", None)) + return f"{v:.6g}", float(v) + item = t.item(row, col) + return (item.text() if item is not None else ""), None + + def _block_matcher(self, row: int, filters) -> bool: + t = self._blocks_table + bid = self._row_key(t, row) + for col, f in filters: + text, raw = self._block_cell(t, row, col, bid) + if not f(text, raw): + return False + return True + + def _sparing_matcher(self, row: int, filters) -> bool: + from raptor.core.fileio.typed import SPARING_TYPE_TO_FILE_STRING + t = self._sparing_table + bid = self._row_key(t, row) + b = self._view.blocks.get(bid) if self._view is not None \ + and bid is not None else None + for col, f in filters: + if col == 2 and b is not None: + text = SPARING_TYPE_TO_FILE_STRING.get(b.sparing_type, "None") + else: + item = t.item(row, col) + text = item.text() if item is not None else "" + if not f(text, None): + return False + return True + @staticmethod def _row_key(t: QTableWidget, r: int) -> int | None: """The block id of a row, read from its ID cell (rows move when the @@ -1011,7 +1114,11 @@ def _realize_all(self, t: QTableWidget) -> None: return QApplication.setOverrideCursor(Qt.CursorShape.WaitCursor) try: - self._realize_rows(t, range(t.rowCount())) + # A filtered grid exports (and copies) the rows it shows, so the + # hidden ones need no widgets. + rows = [r for r in range(t.rowCount()) if not t.isRowHidden(r)] \ + if self._filter_active(t) else range(t.rowCount()) + self._realize_rows(t, rows) finally: QApplication.restoreOverrideCursor() diff --git a/raptor/gui/ui/results.py b/raptor/gui/ui/results.py index ad3dadef..b5760aa9 100644 --- a/raptor/gui/ui/results.py +++ b/raptor/gui/ui/results.py @@ -51,6 +51,7 @@ from raptor.core.engine.reports import flow_loss_across_runs from raptor.gui.ui import filedialogs as fdg from raptor.gui.ui.event_log_model import HEADERS, EventLogModel +from raptor.gui.ui.table_filter_bar import attach_if_large from raptor.gui.resources.design_tokens import ThemeManager, SPACING, TYPE, active_colour @@ -1032,6 +1033,9 @@ def _make_table(self, headers: list[str], rows: list[list[str]], "measure, so the true value is at least " "this large.") item.setToolTip(_GT_NOTE + "\n\n" + reason) + # A long table (one row per block in a big model) can be filtered + # by column; a short one is read at a glance and gets no bar. + attach_if_large(table, raw_role=RAW_VALUE_ROLE) return table #: Last details sub-tab the user viewed, by name -- survives diff --git a/raptor/gui/ui/table_filter_bar.py b/raptor/gui/ui/table_filter_bar.py new file mode 100644 index 00000000..a934ac68 --- /dev/null +++ b/raptor/gui/ui/table_filter_bar.py @@ -0,0 +1,415 @@ +"""A filter row under a table's header (Excel's "auto filter"), for big tables. + +``FilterBar`` is attached to a table with ``FilterBar.attach(table)``. It adds +nothing to the layout: one small edit per column sits in an extra strip at +the bottom of the table's own header, so it lines up with the columns, +follows resizing and scrolling, and costs nothing until it is shown. Each +edit takes a small expression (``raptor.gui.table_filter``: text, wildcards, +``>0.9``, ``5..10``, ``blank``...). Typing is debounced, so a filter pass +runs when the typing pauses, not on every keystroke; a pass hides the rows +that do not match with ``setRowHidden`` and copies no data. + +Tables of ``MIN_FILTER_ROWS`` rows or fewer get no filter at all. The strip is +hidden until asked for (Ctrl+F on the table, or the header's right-click +menu). While a filter is active a small "Showing N of M - Clear" note sits at +the table's bottom right. + +A table whose rows are not read from its items (the Data Tables read the +model objects behind the rows, so hidden rows never need their cell widgets) +passes its own ``matcher``. +""" +from __future__ import annotations + +from collections.abc import Callable +from typing import Any + +from PySide6.QtCore import QEvent, QObject, Qt, QTimer, Signal +from PySide6.QtGui import QKeySequence, QShortcut +from PySide6.QtWidgets import ( + QFrame, QHBoxLayout, QLabel, QLineEdit, QMenu, QPushButton, QTableWidget, + QWidget) + +from raptor.gui.table_filter import SYNTAX_HELP, CompiledFilter, compile_filter + +#: A table with this many rows or fewer is small enough to read: no filter. +MIN_FILTER_ROWS = 10 +DEBOUNCE_MS = 150 +_EDIT_H = 24 + +#: (row, [(column, compiled filter)...]) -> does the row stay visible? +Matcher = Callable[[int, "list[tuple[int, CompiledFilter]]"], bool] + + +def _widget_text(w: QWidget | None) -> str: + """The text a cell widget shows (a distribution combo, a parameter field).""" + if w is None: + return "" + for getter in ("currentText", "text"): + fn = getattr(w, getter, None) + if callable(fn): + try: + return str(fn()) + except TypeError: + continue + for child in w.findChildren(QLineEdit): + return child.text() + return "" + + +class FilterBar(QObject): + """The filter strip of one table. See the module docstring.""" + + #: (rows still shown, rows in the table), after every filter pass. + changed = Signal(int, int) + + @classmethod + def attach(cls, table: QTableWidget, **kw: Any) -> FilterBar: + """A bar for `table` (kept alive by the table). Safe to call twice.""" + existing = getattr(table, "_filter_bar", None) + if isinstance(existing, FilterBar): + return existing + bar = cls(table, **kw) + table._filter_bar = bar # type: ignore[attr-defined] + return bar + + def __init__(self, table: QTableWidget, *, matcher: Matcher | None = None, + raw_role: int | None = None, min_rows: int = MIN_FILTER_ROWS, + debounce_ms: int = DEBOUNCE_MS, + filter_columns: set[int] | None = None, + row_key: Callable[[int], Any] | None = None) -> None: + super().__init__(table) + self._table = table + self._matcher = matcher + #: Columns that can be filtered (None = all). The others get a + #: disabled box: their text is not known without their cell widgets. + self._filter_columns = filter_columns + #: Rows whose `row_key` is in `keep_visible` stay shown by a pass the + #: user did not ask for (a refill after an edit), so an edited row + #: that stops matching does not vanish under the cursor. Typing in a + #: filter box clears it: the next pass is the user's. + self._row_key = row_key + self.keep_visible: set[Any] = set() + self._raw_role = raw_role + self._min_rows = min_rows + self._editors: list[QLineEdit] = [] + self._shown = False + self._filters: list[tuple[int, CompiledFilter]] = [] + self._base_h = 0 + self._passes = 0 # filter passes run (the tests count them) + + self._timer = QTimer(self) + self._timer.setSingleShot(True) + self._timer.setInterval(debounce_ms) + self._timer.timeout.connect(self.apply) + + self._refilter = QTimer(self) # coalesces a repopulation's signals + self._refilter.setSingleShot(True) + self._refilter.setInterval(0) + self._refilter.timeout.connect(self._settle) + + self._badge = QFrame(table) + self._badge.setObjectName("FilterBadge") + self._badge.setStyleSheet( + "#FilterBadge { background: palette(window); border: 1px solid " + "palette(mid); border-radius: 4px; }") + lay = QHBoxLayout(self._badge) + lay.setContentsMargins(6, 2, 4, 2) + lay.setSpacing(6) + self._count = QLabel("") + clear = QPushButton("Clear") + clear.setFlat(True) + clear.setCursor(Qt.CursorShape.PointingHandCursor) + clear.clicked.connect(self.clear) + lay.addWidget(self._count) + lay.addWidget(clear) + self._badge.hide() + + header = table.horizontalHeader() + header.setContextMenuPolicy(Qt.ContextMenuPolicy.CustomContextMenu) + header.customContextMenuRequested.connect(self._header_menu) + header.sectionResized.connect(self._place) + header.sectionMoved.connect(self._place) + header.geometriesChanged.connect(self._place) + table.horizontalScrollBar().valueChanged.connect(self._place) + sc = QShortcut(QKeySequence("Ctrl+F"), table) + sc.setContext(Qt.ShortcutContext.WidgetWithChildrenShortcut) + sc.activated.connect(self.toggle) + table.installEventFilter(self) + model = table.model() + # Rows added/removed or reordered (a sort) leave the hidden flags on + # the wrong rows: filter again, once, when the dust settles. + for sig in (model.rowsInserted, model.rowsRemoved, + model.layoutChanged, model.modelReset): + sig.connect(self._model_changed) + for sig in (model.columnsInserted, model.columnsRemoved): + sig.connect(self._columns_changed) + + # ----------------------------------------------------------- availability + def available(self) -> bool: + """Whether this table is big enough to be worth a filter.""" + return self._table.rowCount() > self._min_rows + + def is_shown(self) -> bool: + return self._shown + + def is_active(self) -> bool: + return any(not f.empty and f.valid for _c, f in self._filters) + + def toggle(self) -> None: + self.set_shown(not self._shown) + + def set_shown(self, shown: bool) -> None: + if shown and not self.available(): + return + if shown == self._shown: + return + self._shown = shown + header = self._table.horizontalHeader() + self._grow_fixed_height(_EDIT_H if shown else -_EDIT_H) + if shown: + self._build_editors() + self._base_h = header.sizeHint().height() + header.setFixedHeight(self._base_h + _EDIT_H) + header.setViewportMargins(0, 0, 0, _EDIT_H) + for e in self._editors: + e.show() + self._place() + if self._editors: + self._editors[0].setFocus() + else: + self._timer.stop() + for e in self._editors: + e.blockSignals(True) + e.clear() + e.blockSignals(False) + e.setStyleSheet("") + e.hide() + self._filters = [] + self._show_all_rows() + header.setViewportMargins(0, 0, 0, 0) + header.setMinimumHeight(0) + header.setMaximumHeight(16777215) + header.resize(header.width(), self._base_h) + header.updateGeometry() + self._table.updateGeometries() + self._update_badge(self._table.rowCount(), self._table.rowCount()) + self._table.viewport().update() + + def _grow_fixed_height(self, delta: int) -> None: + """A table sized to exactly fit its rows (the results tables) needs + the strip's height added, or the last row is clipped.""" + t = self._table + if t.maximumHeight() < 16777215: + t.setMaximumHeight(t.maximumHeight() + delta) + if t.minimumHeight() > 0: + t.setMinimumHeight(t.minimumHeight() + delta) + + # --------------------------------------------------------------- editors + def _build_editors(self) -> None: + ncols = self._table.columnCount() + while len(self._editors) < ncols: + c = len(self._editors) + e = QLineEdit(self._table.horizontalHeader()) + e.setClearButtonEnabled(True) + e.setPlaceholderText("filter") + e.setToolTip(SYNTAX_HELP) + if self._filter_columns is not None \ + and c not in self._filter_columns: + e.setEnabled(False) + e.setPlaceholderText("") + e.setToolTip("This column cannot be filtered.") + e.setAccessibleName(f"Filter column {c + 1}") + e.textChanged.connect(self._typed) + self._editors.append(e) + for c, e in enumerate(self._editors): + h = self._table.horizontalHeaderItem(c) + e.setAccessibleName(f"Filter {h.text()}" if h is not None + else f"Filter column {c + 1}") + + def _place(self, *_a: object) -> None: + if not self._shown: + return + header = self._table.horizontalHeader() + y = max(header.height() - _EDIT_H, 0) + for c, e in enumerate(self._editors): + if c >= self._table.columnCount() or header.isSectionHidden(c): + e.hide() + continue + x = header.sectionViewportPosition(c) + w = header.sectionSize(c) + e.setGeometry(x, y, max(w, 1), _EDIT_H) + e.setVisible(w > 8) + + # ------------------------------------------------------------- filtering + def _typed(self, *_a: object) -> None: + """A keystroke: (re)start the debounce; the pass runs once it pauses.""" + self.keep_visible.clear() + self._timer.start() + + def _model_changed(self, *_a: object) -> None: + if self._shown: + self._refilter.start() # one pass for a burst of changes + + def _settle(self) -> None: + """After a refill or sort has finished (not in the middle of one).""" + if not self._shown: + return + if not self.available(): + self.set_shown(False) # shrank to a table worth no filter + elif self.is_active(): + self.apply() + else: + self._place() + + def _columns_changed(self, *_a: object) -> None: + """A refill with other columns: one edit per column again.""" + if not self._shown: + return + self._build_editors() + for c, e in enumerate(self._editors): + e.setVisible(c < self._table.columnCount()) + self._place() + + def _compile(self) -> list[tuple[int, CompiledFilter]]: + out = [] + for c, e in enumerate(self._editors[:self._table.columnCount()]): + f = compile_filter(e.text()) + if not e.isEnabled(): + continue + e.setStyleSheet("" if f.valid else + "QLineEdit { border: 1px solid #c0392b; }") + if not f.valid: + e.setToolTip(f"{f.message}\n\n{SYNTAX_HELP}") + else: + e.setToolTip(SYNTAX_HELP) + if f.valid and not f.empty: + out.append((c, f)) + return out + + def apply(self) -> None: + """Run a filter pass now (the debounce calls this).""" + self._passes += 1 + self._filters = self._compile() + table = self._table + n = table.rowCount() + table.setUpdatesEnabled(False) + try: + shown = 0 + for r in range(n): + keep = (not self._filters) or self._row_matches(r) \ + or self._pinned(r) + if table.isRowHidden(r) == keep: + table.setRowHidden(r, not keep) + shown += keep + finally: + table.setUpdatesEnabled(True) + self._update_badge(shown, n) + self.changed.emit(shown, n) + + def _pinned(self, row: int) -> bool: + return bool(self.keep_visible and self._row_key is not None + and self._row_key(row) in self.keep_visible) + + def _row_matches(self, row: int) -> bool: + if self._matcher is not None: + return self._matcher(row, self._filters) + return self.item_matcher(row, self._filters) + + def item_matcher(self, row: int, + filters: list[tuple[int, CompiledFilter]]) -> bool: + """The default: read each filtered column's item (or cell widget).""" + table = self._table + for c, f in filters: + item = table.item(row, c) + if item is not None: + text = item.text() + raw = None + if self._raw_role is not None: + v = item.data(self._raw_role) + if isinstance(v, (int, float)) and not isinstance(v, bool): + raw = float(v) + if not text and item.flags() & Qt.ItemFlag.ItemIsUserCheckable: + text = ("yes" if item.checkState() + == Qt.CheckState.Checked else "no") + else: + text, raw = _widget_text(table.cellWidget(row, c)), None + if not f(text, raw): + return False + return True + + def clear(self) -> None: + for e in self._editors: + e.blockSignals(True) + e.clear() + e.blockSignals(False) + self._timer.stop() + self.apply() + + def rows_shown_of_total(self) -> tuple[int, int]: + """(rows a filter leaves showing, rows in the table).""" + return len(self.visible_rows()), self._table.rowCount() + + def visible_rows(self) -> list[int]: + """The rows a filter leaves showing, top to bottom.""" + t = self._table + return [r for r in range(t.rowCount()) if not t.isRowHidden(r)] + + def _show_all_rows(self) -> None: + t = self._table + t.setUpdatesEnabled(False) + try: + for r in range(t.rowCount()): + if t.isRowHidden(r): + t.setRowHidden(r, False) + finally: + t.setUpdatesEnabled(True) + + # ----------------------------------------------------------------- badge + def _update_badge(self, shown: int, total: int) -> None: + if self.is_active(): + self._count.setText(f"Showing {shown:,} of {total:,}") + self._badge.adjustSize() + self._badge.show() + self._badge.raise_() + self._position_badge() + else: + self._badge.hide() + + def _position_badge(self) -> None: + t = self._table + bar = t.horizontalScrollBar() + y_off = bar.height() if bar.isVisible() else 0 + self._badge.move( + max(t.viewport().width() + t.verticalHeader().width() + - self._badge.width() - 6, 0), + max(t.height() - self._badge.height() - y_off - 6, 0)) + + def eventFilter(self, obj: QObject, ev: QEvent) -> bool: # noqa: N802 + if obj is self._table and ev.type() == QEvent.Type.Resize \ + and self._badge.isVisible(): + self._position_badge() + return False + + # ------------------------------------------------------------------ menu + def _header_menu(self, pos: Any) -> None: + if not self.available(): + return + menu = QMenu(self._table) + act = menu.addAction("Filter rows") + act.setCheckable(True) + act.setChecked(self._shown) + act.setShortcut(QKeySequence("Ctrl+F")) + act.triggered.connect(lambda _c=False: self.toggle()) + if self.is_active(): + menu.addAction("Clear filters").triggered.connect( + lambda _c=False: self.clear()) + menu.exec(self._table.horizontalHeader().mapToGlobal(pos)) + + +def attach_if_large(table: QTableWidget, **kw: Any) -> FilterBar | None: + """A filter bar for `table` when it has more than ``MIN_FILTER_ROWS`` rows, + else nothing. (The bar re-checks as the table is refilled.)""" + if table.rowCount() <= kw.get("min_rows", MIN_FILTER_ROWS): + return None + return FilterBar.attach(table, **kw) + diff --git a/tests/unit/test_gui_table_filter_bar.py b/tests/unit/test_gui_table_filter_bar.py new file mode 100644 index 00000000..a3ea5b9c --- /dev/null +++ b/tests/unit/test_gui_table_filter_bar.py @@ -0,0 +1,310 @@ +"""The filter row under big tables: debounce, N of M, small tables, repopulate, +the results-table hook and the Data Tables (filtering the model, not widgets).""" +from __future__ import annotations + +import time + +import pytest +from PySide6.QtCore import Qt +from PySide6.QtTest import QTest +from PySide6.QtWidgets import QApplication, QTableWidget, QTableWidgetItem + +from raptor.gui.ui.table_filter_bar import MIN_FILTER_ROWS, FilterBar + +pytestmark = pytest.mark.gui + + +@pytest.fixture(scope="module", autouse=True) +def _app(): + return QApplication.instance() or QApplication([]) + + +def _table(n: int, cols: int = 3) -> QTableWidget: + t = QTableWidget(n, cols) + t.setHorizontalHeaderLabels([f"C{c}" for c in range(cols)]) + for r in range(n): + t.setItem(r, 0, QTableWidgetItem(f"Pump_{r}")) + for c in range(1, cols): + it = QTableWidgetItem(f"{(r * 7 % 100) / 100:.2f}") + it.setData(Qt.ItemDataRole.UserRole + 1, (r * 7 % 100) / 100) + t.setItem(r, c, it) + t.resize(500, 300) + t.show() + QApplication.processEvents() + return t + + +def _wait(bar: FilterBar) -> None: + QTest.qWait(bar._timer.interval() + 60) + + +def test_small_tables_get_no_filter(): + t = _table(MIN_FILTER_ROWS) + bar = FilterBar.attach(t) + assert not bar.available() + bar.set_shown(True) + assert not bar.is_shown() + assert FilterBar.attach(_table(MIN_FILTER_ROWS + 1)).available() + + +def test_typing_is_debounced_to_one_pass(): + t = _table(200) + bar = FilterBar.attach(t, raw_role=Qt.ItemDataRole.UserRole + 1) + bar.set_shown(True) + assert bar._editors and bar._passes == 0 + for ch in "Pump_1": + bar._editors[0].setText(bar._editors[0].text() + ch) + QTest.qWait(10) + assert bar._passes == 0 # still typing + _wait(bar) + assert bar._passes == 1 + shown, total = bar.rows_shown_of_total() + assert total == 200 and shown == 111 # Pump_1, Pump_10-19, Pump_100-199 + + +def test_numeric_and_count_label_and_clear(): + t = _table(200) + bar = FilterBar.attach(t, raw_role=Qt.ItemDataRole.UserRole + 1) + bar.set_shown(True) + bar._editors[1].setText(">0.5") + _wait(bar) + shown, total = bar.rows_shown_of_total() + assert 0 < shown < total + assert bar._badge.isVisible() and f"{shown:,} of {total:,}" in bar._count.text() + bar.clear() + assert bar.rows_shown_of_total() == (200, 200) + assert not bar._badge.isVisible() + + +def test_bad_expression_leaves_rows_and_flags_the_box(): + t = _table(50) + bar = FilterBar.attach(t) + bar.set_shown(True) + bar._editors[1].setText(">abc") + _wait(bar) + assert bar.rows_shown_of_total() == (50, 50) + assert "border" in bar._editors[1].styleSheet() + + +def test_hiding_the_strip_clears_the_filter(): + t = _table(50) + bar = FilterBar.attach(t) + bar.set_shown(True) + bar._editors[0].setText("Pump_4*") + _wait(bar) + assert bar.rows_shown_of_total()[0] < 50 + bar.set_shown(False) + assert bar.rows_shown_of_total() == (50, 50) + + +def test_filter_survives_a_repopulate_and_a_sort(): + t = _table(60) + bar = FilterBar.attach(t) + bar.set_shown(True) + bar._editors[0].setText("Pump_5*") + _wait(bar) + first = bar.rows_shown_of_total()[0] + t.sortItems(0, Qt.SortOrder.DescendingOrder) + QTest.qWait(60) + assert bar.rows_shown_of_total()[0] == first + assert all(t.item(r, 0).text().startswith("Pump_5") + for r in bar.visible_rows()) + t.setRowCount(0) + t.setRowCount(70) + for r in range(70): + t.setItem(r, 0, QTableWidgetItem(f"Pump_{r}")) + QTest.qWait(60) + assert all(t.item(r, 0).text().startswith("Pump_5") + for r in bar.visible_rows()) + + +def test_a_table_that_shrinks_below_the_threshold_closes_the_strip(): + t = _table(40) + bar = FilterBar.attach(t) + bar.set_shown(True) + t.setRowCount(5) + QTest.qWait(30) + assert not bar.is_shown() + + +def test_filtering_30000_rows_is_fast(): + t = _table(30000, cols=4) + bar = FilterBar.attach(t, raw_role=Qt.ItemDataRole.UserRole + 1) + bar.set_shown(True) + bar._editors[0].setText("Pump_1*") + bar._editors[2].setText(">0.3 <0.9") + t0 = time.perf_counter() + bar.apply() + assert time.perf_counter() - t0 < 2.0 # ~0.1 s; generous for slow CI + shown, total = bar.rows_shown_of_total() + assert 0 < shown < total + + +def test_results_tables_get_a_bar_only_when_long(monkeypatch): + from raptor.gui.ui.results import ResultsDock + make = ResultsDock._make_table + owner = ResultsDock.__new__(ResultsDock) + small = make(owner, ["A", "B"], [[f"r{i}", str(i)] for i in range(MIN_FILTER_ROWS)]) + big = make(owner, ["A", "B"], [[f"r{i}", (f"{i:.2f}", float(i))] for i in range(60)]) + assert getattr(small, "_filter_bar", None) is None + bar = big._filter_bar + bar.set_shown(True) + bar._editors[1].setText(">=50") + _wait(bar) + assert bar.rows_shown_of_total() == (10, 60) + # the table still shows every row it had in full once the strip closes, + # and the fixed-height table grew by the strip and shrank back + h = big.maximumHeight() + bar.set_shown(False) + assert big.maximumHeight() == h - 24 + assert bar.rows_shown_of_total() == (60, 60) + + +# ---------------------------------------------------------------- Data Tables +N = 150 + + +def _model(n: int = N): + from raptor.api.build import build_from_spec + blocks = [{"name": f"B{i:03d}"} for i in range(n)] + links = ([{"from": "Start", "to": "B000"}] + + [{"from": f"B{i:03d}", "to": f"B{i + 1:03d}"} for i in range(n - 1)] + + [{"from": f"B{n - 1:03d}", "to": "End"}]) + return build_from_spec({ + "name": "Big", "blocks": blocks, + "nodes": [{"id": 1, "name": "Start", "type": "start"}, + {"id": 2, "name": "End", "type": "end"}], + "links": links, "system": {"term_condition": 100, "num_runs": 1}}, + auto_layout=True) + + +@pytest.fixture +def win(): + from raptor.gui.ui import settings + from raptor.gui.ui.main_window import MainWindow + settings.set_value("startup/show_start_page", False) + w = MainWindow() + w.resize(1500, 900) + w.show() + w._load_into_app(_model(), None) + QApplication.processEvents() + w._data_tables.refresh(force=True) + QApplication.processEvents() + yield w + w._dirty = False + w.close() + + +def _built(t, col=2): + return [r for r in range(t.rowCount()) if t.cellWidget(r, col) is not None] + + +def test_data_tables_filter_reads_the_model_not_the_widgets(win): + dock = win._data_tables + t = dock._blocks_table + bar = dock._filter_bars[t] + before = len(_built(t)) + assert before < N + bar.set_shown(True) + bar._editors[0].setText("B1*") # B100..B149 + B1xx names + _wait(bar) + shown, total = bar.rows_shown_of_total() + assert total == N and shown == 50 + # The matching rows that are in view got widgets; the far ones did not. + far = [r for r in bar.visible_rows() if r > N - 5] + assert far and all(t.cellWidget(r, 2) is None for r in far) + # widget columns filter without widgets: every row shares one failure dist + from raptor.gui.ui.distro_combo import distro_display_name + name = distro_display_name( + dock._view.blocks[1].fail_distro, is_failure=True) + bar._editors[0].setText("") + bar._editors[2].setText(name) + _wait(bar) + assert bar.rows_shown_of_total()[0] == N + bar._editors[2].setText("!" + name) + _wait(bar) + assert bar.rows_shown_of_total()[0] == 0 + + +def test_data_tables_filter_on_a_parameter_value(win): + dock = win._data_tables + t = dock._blocks_table + view = dock._view + # make two blocks distinct + first = next(iter(sorted(view.blocks))) + view.blocks[first].fail_values[0] = 12345.0 + dock.refresh(force=True) + QApplication.processEvents() + bar = dock._filter_bars[t] + bar.set_shown(True) + bar._editors[3].setText("12345") + _wait(bar) + assert bar.rows_shown_of_total()[0] == 1 + + +def test_sorting_still_works_while_filtered(win): + dock = win._data_tables + t = dock._blocks_table + bar = dock._filter_bars[t] + bar.set_shown(True) + bar._editors[0].setText("B0*") + _wait(bar) + n = bar.rows_shown_of_total()[0] + t.sortItems(0, Qt.SortOrder.DescendingOrder) + QTest.qWait(80) + rows = bar.visible_rows() + assert len(rows) == n + names = [t.item(r, 0).text() for r in rows] + assert names == sorted(names, reverse=True) + + +def test_export_reads_only_the_visible_rows(win, monkeypatch, tmp_path): + from raptor.gui.ui import data_tables as dt + dock = win._data_tables + t = dock._blocks_table + dock._tabs.setCurrentWidget(t) + bar = dock._filter_bars[t] + bar.set_shown(True) + bar._editors[0].setText("B07?") + _wait(bar) + out = tmp_path / "blocks.csv" + titles = [] + monkeypatch.setattr(dt.fdg, "getSaveFileName", + staticmethod(lambda p, title, *a, **k: + (titles.append(title) or str(out), ""))) + dock._export_current_table_csv() + lines = out.read_text().strip().splitlines() + assert len(lines) == 1 + 10 # header + B070..B079 + assert "10 of 150" in titles[0] + # hidden rows never had to be built for it + assert t.cellWidget(N - 1, 2) is None + + +def test_an_edited_row_that_stops_matching_stays_until_the_next_pass(win): + dock = win._data_tables + t = dock._blocks_table + bar = dock._filter_bars[t] + bar.set_shown(True) + bar._editors[3].setText("<=100") + _wait(bar) + shown0 = bar.rows_shown_of_total()[0] + assert shown0 > 0 + row = bar.visible_rows()[0] + t.setCurrentCell(row, 0) + t.selectRow(row) + bid = dock._row_key(t, row) + dock._view.blocks[bid].fail_values[0] = 99999.0 # no longer <=100 + dock.refresh(force=True) + QTest.qWait(60) + keys = {dock._row_key(t, r) for r in bar.visible_rows()} + assert bid in keys # kept under the cursor + bar._editors[3].setText("<=100 ") # the user types again + _wait(bar) + keys = {dock._row_key(t, r) for r in bar.visible_rows()} + assert bid not in keys + + +def test_small_data_tables_have_no_strip(win): + dock = win._data_tables + bar = dock._filter_bars[dock._nodes_table] # Start + End only + assert not bar.available() diff --git a/tests/unit/test_table_filter.py b/tests/unit/test_table_filter.py new file mode 100644 index 00000000..fc75c6df --- /dev/null +++ b/tests/unit/test_table_filter.py @@ -0,0 +1,90 @@ +"""The filter-box expression language (pure Python).""" +from __future__ import annotations + +import time + +import pytest + +from raptor.gui.table_filter import compile_filter, parse_number + + +def m(expr, text, raw=None): + f = compile_filter(expr) + assert f.valid, f.message + return f(text, raw) + + +@pytest.mark.parametrize("expr,text,want", [ + ("pump", "Main Pump A", True), ("PUMP", "pump", True), ("pump", "valve", False), + ('"main pump"', "The Main Pump", True), ('"main pump"', "Main Pump", False), + ("Pump*", "Pump_A", True), ("Pump*", "Main Pump", False), + ("*_B", "Pump_B", True), ("*_B", "Pump_B2", False), + ("*pump*", "the Pump room", True), ("P?mp", "Pump", True), ("P?mp", "Pomp2", False), + ("=Pump_A", "pump_a", True), ("=Pump_A", "Pump_AB", False), + ("!pump", "valve", True), ("!pump", "Pump", False), + ("blank", "", True), ("empty", " ", True), ("blank", "x", False), + ("nonblank", "x", True), ("nonblank", "", False), +]) +def test_text_forms(expr, text, want): + assert m(expr, text) is want + + +@pytest.mark.parametrize("expr,text,raw,want", [ + (">0.9", "0.95", None, True), (">0.9", "0.9", None, False), + (">=0.9", "0.9", None, True), ("<5", "4", None, True), ("<=5", "5", None, True), + ("<5", "7", None, False), ("=3", "3.0", None, True), ("!=3", "4", None, True), + ("!=3", "3", None, False), ("5..10", "7", None, True), ("5..10", "10", None, True), + ("10..5", "7", None, True), ("5..10", "11", None, False), + (">100", "1,234.5", None, True), (">90", "96.2%", None, True), + (">1000", ">5000", None, True), ("<10", "3 hours", None, True), + (">5", "abc", None, False), (">5", "", None, False), + (">5", "shown as text", 9.0, True), # the raw value wins + (">5", "9", 2.0, False), + ("=Pump", "Pump", None, True), +]) +def test_number_forms(expr, text, raw, want): + assert m(expr, text, raw) is want + + +def test_and_or_precedence(): + f = compile_filter(">5 <10 | blank") + assert f("7", None) and f("", None) + assert not f("12", None) and not f("3", None) + g = compile_filter("pump, valve") + assert g("Pump", None) and g("valve 2", None) and not g("fan", None) + assert compile_filter("pump !valve")("Pump A", None) + assert not compile_filter("pump !valve")("Pump valve", None) + + +def test_quoted_separators_are_literal(): + f = compile_filter('"a|b"') + assert f("xa|by", None) and not f("a", None) + + +@pytest.mark.parametrize("bad", ['"unclosed', ">", ">abc", "5..x", "!", "<="]) +def test_bad_input_never_raises_and_matches_everything(bad): + f = compile_filter(bad) + assert not f.valid and f.message + assert f("anything", None) and f("", None) + + +def test_empty_expression_is_empty_and_matches_all(): + f = compile_filter(" ") + assert f.empty and f.valid and f("x", None) + + +def test_parse_number(): + assert parse_number("1,234.5") == 1234.5 + assert parse_number("96.2%") == 96.2 + assert parse_number("-3e2") == -300.0 + assert parse_number("Pump 3") is None + assert parse_number("") is None + + +def test_filtering_a_large_column_is_fast(): + cells = [(f"Block_{i}", float(i)) for i in range(30000)] + f = compile_filter(">100 <20000 Block_1*") + t0 = time.perf_counter() + n = sum(1 for t, r in cells if f(t, r)) + assert time.perf_counter() - t0 < 1.0 + assert n > 0 From ed690fa69b6036b90658d153ceb42687a3d72d21 Mon Sep 17 00:00:00 2001 From: Daniel Mulholland Date: Fri, 6 Nov 2026 22:01:50 +1000 Subject: [PATCH 2/6] Filter bar: satisfy mypy --- raptor/gui/table_filter.py | 4 +++- raptor/gui/ui/data_tables.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/raptor/gui/table_filter.py b/raptor/gui/table_filter.py index e76d0c5c..ea720967 100644 --- a/raptor/gui/table_filter.py +++ b/raptor/gui/table_filter.py @@ -83,7 +83,9 @@ def _cell_number(text: str, raw: float | None) -> float | None: def _split_or(expr: str) -> list[str]: """Split on | and , outside double quotes.""" - parts, cur, quoted = [], [], False + parts: list[str] = [] + cur: list[str] = [] + quoted = False for ch in expr: if ch == '"': quoted = not quoted diff --git a/raptor/gui/ui/data_tables.py b/raptor/gui/ui/data_tables.py index 82da8878..4c77d592 100644 --- a/raptor/gui/ui/data_tables.py +++ b/raptor/gui/ui/data_tables.py @@ -919,7 +919,7 @@ def _export_current_table_csv(self) -> None: input data.""" import csv table = self._tabs.currentWidget() - if table is None or not hasattr(table, "rowCount"): + if not isinstance(table, QTableWidget): return tab_name = self._tabs.tabText(self._tabs.currentIndex()) default = tab_name.replace(" ", "_").lower() + ".csv" From b764759d93b6f6a6259244dedd6be4cabb23e63b Mon Sep 17 00:00:00 2001 From: Daniel Mulholland Date: Sat, 7 Nov 2026 18:46:45 +1000 Subject: [PATCH 3/6] Filter bar: hold the table weakly so a shown table is freed by refcount, not mid-collection --- raptor/gui/ui/table_filter_bar.py | 29 +++++++++++++++++++------ tests/unit/test_gui_table_filter_bar.py | 20 +++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/raptor/gui/ui/table_filter_bar.py b/raptor/gui/ui/table_filter_bar.py index a934ac68..5870a7de 100644 --- a/raptor/gui/ui/table_filter_bar.py +++ b/raptor/gui/ui/table_filter_bar.py @@ -20,6 +20,7 @@ """ from __future__ import annotations +import weakref from collections.abc import Callable from typing import Any @@ -62,6 +63,13 @@ class FilterBar(QObject): #: (rows still shown, rows in the table), after every filter pass. changed = Signal(int, int) + @property + def _table(self) -> QTableWidget: + table = self._table_ref() + if table is None: + raise RuntimeError("the filtered table is gone") + return table + @classmethod def attach(cls, table: QTableWidget, **kw: Any) -> FilterBar: """A bar for `table` (kept alive by the table). Safe to call twice.""" @@ -78,7 +86,11 @@ def __init__(self, table: QTableWidget, *, matcher: Matcher | None = None, filter_columns: set[int] | None = None, row_key: Callable[[int], Any] | None = None) -> None: super().__init__(table) - self._table = table + # A weak reference: the table owns the bar (`table._filter_bar`), so a + # strong one back makes a Python cycle that only the cyclic collector + # frees -- and it then tears a shown table down mid-collection, with + # this object's Python event filter still installed on it. + self._table_ref = weakref.ref(table) self._matcher = matcher #: Columns that can be filtered (None = all). The others get a #: disabled box: their text is not known without their cell widgets. @@ -227,7 +239,7 @@ def _build_editors(self) -> None: else f"Filter column {c + 1}") def _place(self, *_a: object) -> None: - if not self._shown: + if not self._shown or self._table_ref() is None: return header = self._table.horizontalHeader() y = max(header.height() - _EDIT_H, 0) @@ -247,12 +259,12 @@ def _typed(self, *_a: object) -> None: self._timer.start() def _model_changed(self, *_a: object) -> None: - if self._shown: + if self._shown and self._table_ref() is not None: self._refilter.start() # one pass for a burst of changes def _settle(self) -> None: """After a refill or sort has finished (not in the middle of one).""" - if not self._shown: + if not self._shown or self._table_ref() is None: return if not self.available(): self.set_shown(False) # shrank to a table worth no filter @@ -263,7 +275,7 @@ def _settle(self) -> None: def _columns_changed(self, *_a: object) -> None: """A refill with other columns: one edit per column again.""" - if not self._shown: + if not self._shown or self._table_ref() is None: return self._build_editors() for c, e in enumerate(self._editors): @@ -385,8 +397,11 @@ def _position_badge(self) -> None: max(t.height() - self._badge.height() - y_off - 6, 0)) def eventFilter(self, obj: QObject, ev: QEvent) -> bool: # noqa: N802 - if obj is self._table and ev.type() == QEvent.Type.Resize \ - and self._badge.isVisible(): + # Called while the table is being torn down too: touch nothing then. + if ev.type() != QEvent.Type.Resize: + return False + table = self._table_ref() + if table is not None and obj is table and self._badge.isVisible(): self._position_badge() return False diff --git a/tests/unit/test_gui_table_filter_bar.py b/tests/unit/test_gui_table_filter_bar.py index a3ea5b9c..e8f74d1d 100644 --- a/tests/unit/test_gui_table_filter_bar.py +++ b/tests/unit/test_gui_table_filter_bar.py @@ -308,3 +308,23 @@ def test_small_data_tables_have_no_strip(win): dock = win._data_tables bar = dock._filter_bars[dock._nodes_table] # Start + End only assert not bar.available() + + +def test_a_filtered_table_is_freed_without_the_cyclic_collector(): + """The table owns its bar, so the bar must not own the table back: a + Python cycle is freed by `gc` mid-run, which tears a shown table down with + the bar's event filter still on it (a segfault in CI, in whichever test + happened to trigger the collection).""" + import gc + import weakref + gc.collect() + gc.disable() + try: + t = _table(40) + bar = FilterBar.attach(t) + bar.set_shown(True) + t_ref, bar_ref = weakref.ref(t), weakref.ref(bar) + del t, bar + assert t_ref() is None and bar_ref() is None + finally: + gc.enable() From b1331aafd8c7fd7a0bf73ca59f0f89d32b28723a Mon Sep 17 00:00:00 2001 From: Daniel Mulholland Date: Sat, 7 Nov 2026 19:43:44 +1000 Subject: [PATCH 4/6] DEBUG: remove - log what the post-test gc.collect() frees --- tests/conftest.py | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 8aaa5404..030e7cc9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -320,6 +320,21 @@ def _refuse(*args, **kwargs): monkeypatch.setattr(cls, fn_name, refuse(cls_name, fn_name)) +_DBG_FH = None + + +def _dbg_out(): # DEBUG: remove + """The xdist controller's stderr (the CI log); pytest's capture would + swallow a crashing worker's own.""" + global _DBG_FH + if _DBG_FH is None: + try: + _DBG_FH = open(f"/proc/{os.getppid()}/fd/2", "w", buffering=1) + except OSError: + _DBG_FH = open(os.devnull, "w") + return _DBG_FH + + @pytest.fixture(autouse=True) def _test_hygiene(request): # Qt window reaping is only meaningful for tests that can create windows; skipping it for @@ -354,8 +369,29 @@ def _test_hygiene(request): # engine -- and tearing a QObject down off the GUI thread segfaults # ("Fatal Python error" under "Garbage-collecting", in unrelated # engine code, in the shard that happened to schedule it there). - gc.collect() - + # DEBUG: remove + import faulthandler as _fh + import sys as _sys + _out = _dbg_out() + _fh.enable(file=_out, all_threads=True) + print(f"GCDBG test={request.node.nodeid}", file=_out, flush=True) + + def _cb(phase, info): + if phase == "start": + print(f"GCDBG start gen={info.get('generation')}", + file=_out, flush=True) + gc.callbacks.append(_cb) + gc.set_debug(gc.DEBUG_COLLECTABLE) + _real = _sys.stderr + _sys.stderr = _out + try: + gc.collect() + finally: + gc.set_debug(0) + _sys.stderr = _real + gc.callbacks.remove(_cb) + print("GCDBG done", file=_out, flush=True) + # END DEBUG # --- automatic `gui` marker ------------------------------------------------------------- From 40e8c5561254166cdd2c5fcc226a77ee4a24bcc4 Mon Sep 17 00:00:00 2001 From: Daniel Mulholland Date: Sat, 7 Nov 2026 19:56:51 +1000 Subject: [PATCH 5/6] Canvas: the view owns its scene, so a dropped view is freed in a safe order --- raptor/gui/ui/diagram.py | 6 ++++++ tests/conftest.py | 40 ++-------------------------------------- 2 files changed, 8 insertions(+), 38 deletions(-) diff --git a/raptor/gui/ui/diagram.py b/raptor/gui/ui/diagram.py index c86e2bff..e9d8066c 100644 --- a/raptor/gui/ui/diagram.py +++ b/raptor/gui/ui/diagram.py @@ -235,6 +235,12 @@ class DiagramView(ContextMenuMixin, LiveOverlayMixin, QGraphicsView): def __init__(self, parent=None): self._scene = QGraphicsScene() super().__init__(self._scene, parent) + # The scene belongs to the view in Qt's tree, not to this Python + # attribute alone: otherwise a view dropped without close() is freed + # by the cyclic garbage collector, which may destroy the scene (and + # its items) before the view, and the view then paints or detaches + # from a freed scene -- a segfault inside gc.collect(). + self._scene.setParent(self) # This canvas's grid-snap step, read by its items through their # scene. It was one module-wide dict, so two open windows would # have snapped to each other's grid. diff --git a/tests/conftest.py b/tests/conftest.py index 030e7cc9..8aaa5404 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -320,21 +320,6 @@ def _refuse(*args, **kwargs): monkeypatch.setattr(cls, fn_name, refuse(cls_name, fn_name)) -_DBG_FH = None - - -def _dbg_out(): # DEBUG: remove - """The xdist controller's stderr (the CI log); pytest's capture would - swallow a crashing worker's own.""" - global _DBG_FH - if _DBG_FH is None: - try: - _DBG_FH = open(f"/proc/{os.getppid()}/fd/2", "w", buffering=1) - except OSError: - _DBG_FH = open(os.devnull, "w") - return _DBG_FH - - @pytest.fixture(autouse=True) def _test_hygiene(request): # Qt window reaping is only meaningful for tests that can create windows; skipping it for @@ -369,29 +354,8 @@ def _test_hygiene(request): # engine -- and tearing a QObject down off the GUI thread segfaults # ("Fatal Python error" under "Garbage-collecting", in unrelated # engine code, in the shard that happened to schedule it there). - # DEBUG: remove - import faulthandler as _fh - import sys as _sys - _out = _dbg_out() - _fh.enable(file=_out, all_threads=True) - print(f"GCDBG test={request.node.nodeid}", file=_out, flush=True) - - def _cb(phase, info): - if phase == "start": - print(f"GCDBG start gen={info.get('generation')}", - file=_out, flush=True) - gc.callbacks.append(_cb) - gc.set_debug(gc.DEBUG_COLLECTABLE) - _real = _sys.stderr - _sys.stderr = _out - try: - gc.collect() - finally: - gc.set_debug(0) - _sys.stderr = _real - gc.callbacks.remove(_cb) - print("GCDBG done", file=_out, flush=True) - # END DEBUG + gc.collect() + # --- automatic `gui` marker ------------------------------------------------------------- From f14e96b706b2ddaf9dfbebf299836e77ce844ed4 Mon Sep 17 00:00:00 2001 From: Daniel Mulholland Date: Sat, 7 Nov 2026 20:09:24 +1000 Subject: [PATCH 6/6] Tool-cursor tests delete their canvas with Qt instead of leaving it to the garbage collector; undo the scene-parenting experiment --- raptor/gui/ui/diagram.py | 6 ------ tests/unit/test_gui_tool_cursor.py | 25 +++++++++++++++++-------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/raptor/gui/ui/diagram.py b/raptor/gui/ui/diagram.py index e9d8066c..c86e2bff 100644 --- a/raptor/gui/ui/diagram.py +++ b/raptor/gui/ui/diagram.py @@ -235,12 +235,6 @@ class DiagramView(ContextMenuMixin, LiveOverlayMixin, QGraphicsView): def __init__(self, parent=None): self._scene = QGraphicsScene() super().__init__(self._scene, parent) - # The scene belongs to the view in Qt's tree, not to this Python - # attribute alone: otherwise a view dropped without close() is freed - # by the cyclic garbage collector, which may destroy the scene (and - # its items) before the view, and the view then paints or detaches - # from a freed scene -- a segfault inside gc.collect(). - self._scene.setParent(self) # This canvas's grid-snap step, read by its items through their # scene. It was one module-wide dict, so two open windows would # have snapped to each other's grid. diff --git a/tests/unit/test_gui_tool_cursor.py b/tests/unit/test_gui_tool_cursor.py index 836384b4..39b871f7 100644 --- a/tests/unit/test_gui_tool_cursor.py +++ b/tests/unit/test_gui_tool_cursor.py @@ -10,7 +10,7 @@ os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") -from PySide6.QtCore import Qt +from PySide6.QtCore import QEvent, Qt from PySide6.QtWidgets import QApplication, QGraphicsSceneHoverEvent from raptor.api.build import build_from_spec @@ -38,6 +38,18 @@ def _view() -> DiagramView: return view +@pytest.fixture +def view(qapp): + """A shown canvas, deleted with Qt (not left to the cyclic collector, + which tore its scene, items and custom cursors down in whatever order + `gc.collect()` reached them and segfaulted CI in the test hygiene).""" + v = _view() + yield v + v.close() + v.deleteLater() + QApplication.sendPostedEvents(None, QEvent.Type.DeferredDelete) + + def _hover(item) -> None: item.hoverEnterEvent(QGraphicsSceneHoverEvent( QGraphicsSceneHoverEvent.Type.GraphicsSceneHoverEnter)) @@ -45,8 +57,7 @@ def _hover(item) -> None: @pytest.mark.parametrize("mode", ["add_block", "add_node", "add_event", "add_hierarchy", "connect"]) -def test_hovering_an_element_keeps_the_tool_cursor(qapp, mode): - view = _view() +def test_hovering_an_element_keeps_the_tool_cursor(view, mode): view.tool_mode = mode tool = view.viewport().cursor() assert tool.shape() == Qt.CursorShape.BitmapCursor @@ -58,8 +69,7 @@ def test_hovering_an_element_keeps_the_tool_cursor(qapp, mode): tool.pixmap().cacheKey() -def test_select_mode_hover_still_shows_the_hand(qapp): - view = _view() +def test_select_mode_hover_still_shows_the_hand(view): view.tool_mode = "add_block" view.tool_mode = "select" block = next(iter(view._block_items.values())) @@ -67,11 +77,10 @@ def test_select_mode_hover_still_shows_the_hand(qapp): assert block.cursor().shape() == Qt.CursorShape.PointingHandCursor -def test_a_middle_button_pan_restores_the_tool_cursor(qapp): +def test_a_middle_button_pan_restores_the_tool_cursor(view): """Releasing a middle-button pan used to set a plain arrow.""" - from PySide6.QtCore import QEvent, QPointF + from PySide6.QtCore import QPointF from PySide6.QtGui import QMouseEvent - view = _view() view.tool_mode = "add_block" point = QPointF(20, 20) view.mousePressEvent(QMouseEvent(