diff --git a/src/eegprep/functions/guifunc/qt.py b/src/eegprep/functions/guifunc/qt.py index 7dbdf9cc..1b2f1457 100644 --- a/src/eegprep/functions/guifunc/qt.py +++ b/src/eegprep/functions/guifunc/qt.py @@ -81,6 +81,8 @@ class QtDialogRenderer: _sync_numeric: Any _select_event_types: Any _select_channels: Any + _navigate_event: Any + _navigate_channel: Any _select_file: Any _open_eegplot: Any _open_rejection_browser: Any @@ -343,6 +345,14 @@ def _connect_callback(self, callback: CallbackSpec | None, widgets: dict[str, An source = widgets.get(params.get("button")) if source is not None: source.clicked.connect(lambda: self._plot_fir_response(source, widgets, params)) + elif callback.name == "navigate_event": + source = widgets.get(params["source"]) + if source is not None: + source.clicked.connect(lambda: self._navigate_event(widgets, params)) + elif callback.name == "navigate_channel": + source = widgets.get(params["source"]) + if source is not None: + source.clicked.connect(lambda: self._navigate_channel(widgets, params)) def _run_tf_cycle_calc(self, button: Any, widgets: dict[str, Any], params: Mapping[str, Any]) -> None: _qt_core, qt_widgets = _require_qt() @@ -974,6 +984,42 @@ def _select_event_types(button: Any, target: Any, params: Mapping[str, Any]) -> target.setText((current + " " + value).strip()) +def _navigate_event(widgets: Mapping[str, Any], params: Mapping[str, Any]) -> None: + _navigate_indexed_form(widgets, params, index_tag="eventnum_tag") + + +def _navigate_channel(widgets: Mapping[str, Any], params: Mapping[str, Any]) -> None: + _navigate_indexed_form(widgets, params, index_tag="channel_tag") + + +def _navigate_indexed_form(widgets: Mapping[str, Any], params: Mapping[str, Any], *, index_tag: str) -> None: + index_widget = widgets.get(params[index_tag]) + if index_widget is None: + return + max_index = max(1, int(params["max_index"])) + try: + current = int(float(_widget_text(index_widget) or "1")) + except ValueError: + current = 1 + new_index = max(1, min(max_index, current + int(params["delta"]))) + index_widget.setText(str(new_index)) + display = params["field_displays"][new_index - 1] + for tag, value in display.items(): + target = widgets.get(tag) + if target is None: + continue + _set_widget_display_value(target, value) + + +def _set_widget_display_value(widget: Any, value: Any) -> None: + if hasattr(widget, "setChecked") and isinstance(value, (bool, np.bool_)): + widget.setChecked(bool(value)) + return + if hasattr(widget, "setText"): + text = "" if value is None or (isinstance(value, str) and value == "") else str(value) + widget.setText(text) + + def _select_channels(button: Any, target: Any, params: Mapping[str, Any]) -> None: channels = [str(value) for value in params.get("channels", ())] if channels: @@ -1390,6 +1436,8 @@ def _firpm_default_devs(amplitudes: list[float]) -> list[float]: "_sync_numeric", "_select_event_types", "_select_channels", + "_navigate_event", + "_navigate_channel", "_select_file", "_open_eegplot", "_open_rejection_browser", diff --git a/src/eegprep/functions/popfunc/pop_chanedit.py b/src/eegprep/functions/popfunc/pop_chanedit.py index e4fa4982..68bcbf22 100644 --- a/src/eegprep/functions/popfunc/pop_chanedit.py +++ b/src/eegprep/functions/popfunc/pop_chanedit.py @@ -10,7 +10,7 @@ from eegprep.functions.adminfunc.eeg_checkset import eeg_checkset from eegprep.functions.guifunc.inputgui import inputgui -from eegprep.functions.guifunc.spec import ControlSpec, DialogSpec +from eegprep.functions.guifunc.spec import CallbackSpec, ControlSpec, DialogSpec from eegprep.functions.popfunc._chanutils import chanlocs_as_list from eegprep.functions.popfunc._pop_utils import format_history_value, parse_key_value_args from eegprep.functions.sigprocfunc.convertlocs import convertlocs @@ -74,6 +74,19 @@ def pop_chanedit_dialog_spec(EEG: dict[str, Any]) -> DialogSpec: ('Index in backup "urchanlocs" structure', "urchan", ""), ("Channel in data array (set=yes)", "datachan", ""), ] + field_displays = tuple(_channel_field_displays(chan, field_rows) for chan in chanlocs) + nav_enabled = len(chanlocs) > 1 + nav_base = { + "channel_tag": "channel", + "max_index": len(chanlocs), + "field_displays": field_displays, + } + + def _nav(tag: str, delta: int) -> CallbackSpec | None: + if not nav_enabled: + return None + return CallbackSpec("navigate_channel", params={**nav_base, "delta": delta, "source": tag}) + controls: list[ControlSpec] = [ ControlSpec('text', 'Channel information ("field_name"):', font_weight="bold"), ControlSpec("spacer"), @@ -109,11 +122,11 @@ def pop_chanedit_dialog_spec(EEG: dict[str, Any]) -> DialogSpec: ControlSpec("spacer"), ControlSpec("spacer"), ControlSpec("pushbutton", "Insert chan", tag="insert_button", enabled=False), - ControlSpec("pushbutton", "<<", tag="back10", enabled=False), - ControlSpec("pushbutton", "<", tag="back1", enabled=False), + ControlSpec("pushbutton", "<<", tag="back10", enabled=nav_enabled, callback=_nav("back10", -10)), + ControlSpec("pushbutton", "<", tag="back1", enabled=nav_enabled, callback=_nav("back1", -1)), ControlSpec("edit", tag="channel", value="1"), - ControlSpec("pushbutton", ">", tag="next1", enabled=False), - ControlSpec("pushbutton", ">>", tag="next10", enabled=False), + ControlSpec("pushbutton", ">", tag="next1", enabled=nav_enabled, callback=_nav("next1", 1)), + ControlSpec("pushbutton", ">>", tag="next10", enabled=nav_enabled, callback=_nav("next10", 10)), ControlSpec("pushbutton", "Append chan", tag="append_button", enabled=False), ControlSpec("pushbutton", "Plot 2-D", tag="plot2d", enabled=False), ControlSpec("text", "Plot radius (0.2-1, []=auto)"), @@ -161,7 +174,7 @@ def pop_chanedit_dialog_spec(EEG: dict[str, Any]) -> DialogSpec: controls=tuple(controls), geomvert=tuple(1 for _row in geometry), known_differences=( - "EEGPrep supports command-line channel edits and one-channel GUI edits; navigation, plotting, and file buttons are visible but disabled.", + "EEGPrep supports command-line channel edits and one-channel-per-submission GUI edits; navigation refreshes the form, while plotting and file buttons remain disabled.", ), ) @@ -191,6 +204,18 @@ def _channel_value_control(field: str, value: Any) -> ControlSpec: return ControlSpec("edit", tag=f"field_{field}", value=_display_value(value)) +def _channel_field_displays(chan: dict[str, Any], field_rows: list[tuple[str, str, str]]) -> dict[str, Any]: + values: dict[str, Any] = {} + for _label, field, _button_label in field_rows: + if field == "urchan": + values["field_urchan"] = _display_value(chan.get("urchan", "")) + elif field == "datachan": + values["field_datachan"] = bool(chan.get("datachan", True)) + else: + values[f"field_{field}"] = _display_channel_value(chan, field) + return values + + def _display_channel_value(chan: dict[str, Any], field: str) -> str: if field not in {"theta", "radius", "sph_theta", "sph_phi", "sph_radius"}: return _display_value(chan.get(field, "")) diff --git a/src/eegprep/functions/popfunc/pop_editeventvals.py b/src/eegprep/functions/popfunc/pop_editeventvals.py index 4e949661..cf5d2702 100644 --- a/src/eegprep/functions/popfunc/pop_editeventvals.py +++ b/src/eegprep/functions/popfunc/pop_editeventvals.py @@ -9,7 +9,7 @@ from eegprep.functions.adminfunc.eeg_checkset import eeg_checkset from eegprep.functions.guifunc.inputgui import inputgui -from eegprep.functions.guifunc.spec import ControlSpec, DialogSpec +from eegprep.functions.guifunc.spec import CallbackSpec, ControlSpec, DialogSpec from eegprep.functions.popfunc._event_utils import event_field_names, events_as_list, normalize_one_based_indices from eegprep.functions.popfunc._pop_utils import format_history_value, is_empty_value as _is_empty from eegprep.functions.popfunc.eeg_lat2point import eeg_lat2point @@ -49,6 +49,22 @@ def pop_editeventvals_dialog_spec(EEG: dict[str, Any]) -> DialogSpec: events = events_as_list(EEG.get("event")) fields = event_field_names(events) current = events[0] if events else {} + field_tags = {field: f"field_{field}" for field in fields} + field_displays = tuple( + {field_tags[field]: _display_event_value(EEG, event, field) for field in fields} for event in events + ) + nav_enabled = len(events) > 1 + _nav_base = { + "eventnum_tag": "eventnum", + "max_index": len(events), + "field_displays": field_displays, + } + + def _nav(tag: str, delta: int) -> CallbackSpec | None: + if not nav_enabled: + return None + return CallbackSpec(name="navigate_event", params={**_nav_base, "delta": delta, "source": tag}) + controls: list[ControlSpec] = [ ControlSpec("text", f"Edit event field values (currently {len(events)} events)", font_weight="bold"), ControlSpec("pushbutton", "Delete event", tag="delete_button", enabled=False), @@ -60,7 +76,7 @@ def pop_editeventvals_dialog_spec(EEG: dict[str, Any]) -> DialogSpec: [ ControlSpec("spacer"), ControlSpec("pushbutton", label, enabled=False), - ControlSpec("edit", tag=f"field_{field}", value=_display_event_value(EEG, current, field)), + ControlSpec("edit", tag=field_tags[field], value=_display_event_value(EEG, current, field)), ControlSpec("spacer"), ] ) @@ -75,11 +91,11 @@ def pop_editeventvals_dialog_spec(EEG: dict[str, Any]) -> DialogSpec: ControlSpec("spacer"), ControlSpec("spacer"), ControlSpec("pushbutton", "Insert event", tag="insert_button", enabled=False), - ControlSpec("pushbutton", "<<", tag="back10", enabled=False), - ControlSpec("pushbutton", "<", tag="back1", enabled=False), + ControlSpec("pushbutton", "<<", tag="back10", enabled=nav_enabled, callback=_nav("back10", -10)), + ControlSpec("pushbutton", "<", tag="back1", enabled=nav_enabled, callback=_nav("back1", -1)), ControlSpec("edit", tag="eventnum", value="1"), - ControlSpec("pushbutton", ">", tag="next1", enabled=False), - ControlSpec("pushbutton", ">>", tag="next10", enabled=False), + ControlSpec("pushbutton", ">", tag="next1", enabled=nav_enabled, callback=_nav("next1", 1)), + ControlSpec("pushbutton", ">>", tag="next10", enabled=nav_enabled, callback=_nav("next10", 10)), ControlSpec("pushbutton", "Append event", tag="append_button", enabled=False), ControlSpec("spacer"), ControlSpec("text", "Original value below", tag="original"), @@ -104,7 +120,7 @@ def pop_editeventvals_dialog_spec(EEG: dict[str, Any]) -> DialogSpec: geometry=tuple(geometry), controls=tuple(controls), known_differences=( - "EEGPrep edits one event per dialog submission; navigation buttons are visible but disabled until richer event-browser parity lands.", + "EEGPrep edits one event per dialog submission; navigation refreshes the form for the targeted event but Delete/Insert/Append remain disabled.", ), ) @@ -175,7 +191,7 @@ def _change_field( def _internal_event_value(output: dict[str, Any], event: dict[str, Any], field: str, value: Any) -> Any: - if field == "latency" and value not in {"", None}: + if field == "latency" and not _is_empty(value): if int(output.get("trials", 1) or 1) > 1: newlat, _ = eeg_lat2point( float(value), @@ -186,7 +202,7 @@ def _internal_event_value(output: dict[str, Any], event: dict[str, Any], field: ) return float(newlat.item()) return (float(value) - float(output.get("xmin", 0))) * float(output.get("srate", 1)) + 1 - if field == "duration" and value not in {"", None}: + if field == "duration" and not _is_empty(value): scale = float(output.get("srate", 1)) / (1000 if int(output.get("trials", 1) or 1) > 1 else 1) return float(value) * scale return value @@ -194,7 +210,7 @@ def _internal_event_value(output: dict[str, Any], event: dict[str, Any], field: def _display_event_value(EEG: dict[str, Any], event: dict[str, Any], field: str) -> Any: value = event.get(field, "") - if value in {"", None}: + if _is_empty(value): return "" if field == "latency": if int(EEG.get("trials", 1) or 1) > 1: diff --git a/tests/test_phase1b_file_edit_pop_functions.py b/tests/test_phase1b_file_edit_pop_functions.py index 9315b40b..f22c0c69 100644 --- a/tests/test_phase1b_file_edit_pop_functions.py +++ b/tests/test_phase1b_file_edit_pop_functions.py @@ -1,6 +1,7 @@ from __future__ import annotations import ast +import os from copy import deepcopy from pathlib import Path @@ -9,12 +10,13 @@ from eegprep.functions.adminfunc.console import _console_python_command from eegprep.functions.adminfunc.eeg_options import EEG_OPTIONS +from eegprep.functions.guifunc.spec import controls_by_tag from eegprep.functions.guifunc.select_multiple_datasets import select_multiple_datasets from eegprep.functions.guifunc.session import EEGPrepSession -from eegprep.functions.popfunc.pop_chanedit import pop_chanedit +from eegprep.functions.popfunc.pop_chanedit import pop_chanedit, pop_chanedit_dialog_spec from eegprep.functions.popfunc.pop_copyset import pop_copyset from eegprep.functions.popfunc.pop_editeventfield import pop_editeventfield -from eegprep.functions.popfunc.pop_editeventvals import pop_editeventvals +from eegprep.functions.popfunc.pop_editeventvals import pop_editeventvals, pop_editeventvals_dialog_spec from eegprep.functions.popfunc.pop_fileio_brainvision_mat import pop_fileio_brainvision_mat from eegprep.functions.popfunc.pop_loadset import pop_loadset from eegprep.functions.popfunc.pop_mergeset import pop_mergeset @@ -302,6 +304,85 @@ def run(self, spec, initial_values=None): assert command == "" +def test_pop_chanedit_navigation_buttons_enabled_with_callbacks(): + eeg = _eeg() + + spec = pop_chanedit_dialog_spec(eeg) + controls = controls_by_tag(spec) + + expected_deltas = {"back10": -10, "back1": -1, "next1": 1, "next10": 10} + for tag, delta in expected_deltas.items(): + control = controls[tag] + assert control.enabled, f"{tag} must be enabled when multiple channels exist" + assert control.callback is not None + assert control.callback.name == "navigate_channel" + assert int(control.callback.params["delta"]) == delta + assert control.callback.params["channel_tag"] == "channel" + assert int(control.callback.params["max_index"]) == len(eeg["chanlocs"]) + displays = control.callback.params["field_displays"] + assert len(displays) == len(eeg["chanlocs"]) + assert displays[1]["field_labels"] == "Pz" + + +def test_pop_chanedit_gui_submits_change_for_navigated_channel(): + class Renderer: + def run(self, spec, initial_values=None): + controls = controls_by_tag(spec) + nav = controls["next1"].callback.params + result = {tag: control.value for tag, control in controls.items()} + result.update(nav["field_displays"][1]) + result["channel"] = "2" + result["field_labels"] = "Oz" + return result + + eeg = _eeg() + + output, command = pop_chanedit(eeg, gui=True, renderer=Renderer(), return_com=True) + + assert output["chanlocs"][0]["labels"] == "Cz" + assert output["chanlocs"][1]["labels"] == "Oz" + assert "'changefield', [2 'labels' 'Oz']" in command + + +def test_qt_renderer_navigation_updates_channel_and_fields(): + os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + pytest.importorskip("PySide6") + from eegprep.functions.guifunc import qt as qt_module + + if qt_module.QtCore is None or qt_module.QtWidgets is None: + pytest.skip("PySide6 Qt libraries unavailable in this environment") + QtDialogRenderer = qt_module.QtDialogRenderer + + eeg = _eeg() + renderer = QtDialogRenderer() + spec = pop_chanedit_dialog_spec(eeg) + _app, dialog, widgets = renderer.build_dialog(spec) + try: + assert widgets["channel"].text() == "1" + widgets["next1"].click() + assert widgets["channel"].text() == "2" + assert widgets["field_labels"].text() == "Pz" + widgets["next10"].click() + assert widgets["channel"].text() == str(len(eeg["chanlocs"])) + widgets["back10"].click() + assert widgets["channel"].text() == "1" + assert widgets["field_labels"].text() == "Cz" + finally: + dialog.close() + + +def test_sample_data_event_and_channel_dialogs_enable_navigation(): + eeg = pop_loadset(SAMPLE_DATASET_PATH) + + event_controls = controls_by_tag(pop_editeventvals_dialog_spec(eeg)) + channel_controls = controls_by_tag(pop_chanedit_dialog_spec(eeg)) + + assert event_controls["next1"].enabled + assert int(event_controls["next1"].callback.params["max_index"]) == len(eeg["event"]) + assert channel_controls["next1"].enabled + assert int(channel_controls["next1"].callback.params["max_index"]) == len(eeg["chanlocs"]) + + def test_pop_chanedit_reads_comma_delimited_ced_with_comments(tmp_path): loc_file = tmp_path / "locs.ced" loc_file.write_text("% exported by EEGLAB\nlabels,X,Y,Z\nFz,0,1,0\nCz,0,0,1\n", encoding="utf-8") diff --git a/tests/test_pop_editeventvals.py b/tests/test_pop_editeventvals.py index 43657cf4..546d90f7 100644 --- a/tests/test_pop_editeventvals.py +++ b/tests/test_pop_editeventvals.py @@ -1,10 +1,12 @@ """Tests for pop_editeventvals latency display/edit symmetry.""" +import os from copy import deepcopy import numpy as np import pytest +from eegprep.functions.guifunc.spec import controls_by_tag from eegprep.functions.popfunc.pop_editeventvals import ( pop_editeventvals, pop_editeventvals_dialog_spec, @@ -71,3 +73,74 @@ def test_epoched_latency_edit_round_trips_through_display(): new_ms = 60.0 out = pop_editeventvals(deepcopy(eeg), "changefield", [1, "latency", new_ms]) assert _display_event_value(out, out["event"][0], "latency") == new_ms + + +def test_navigation_buttons_enabled_with_callbacks(): + """Nav buttons are enabled and carry navigate_event callbacks (#228).""" + eeg = _epoched_eeg() + spec = pop_editeventvals_dialog_spec(eeg) + controls = controls_by_tag(spec) + expected_deltas = {"back10": -10, "back1": -1, "next1": 1, "next10": 10} + for tag, delta in expected_deltas.items(): + control = controls[tag] + assert control.enabled, f"{tag} must be enabled when events exist" + assert control.callback is not None, f"{tag} needs a navigate_event callback" + assert control.callback.name == "navigate_event" + assert int(control.callback.params["delta"]) == delta + assert control.callback.params["eventnum_tag"] == "eventnum" + assert int(control.callback.params["max_index"]) == len(eeg["event"]) + # Per-event display values must cover every event and every field. + displays = control.callback.params["field_displays"] + assert len(displays) == len(eeg["event"]) + assert "field_type" in displays[1] and "field_latency" in displays[1] + + +def test_run_gui_submits_change_for_navigated_event_index(): + """A fake renderer returning eventnum=2 produces a changefield for event 2.""" + eeg = _epoched_eeg() + + class Renderer: + def run(self, spec, initial_values=None): + # Simulate the navigation handler having loaded event 2's display + # values into the field_* widgets, with the user editing 'type'. + controls = controls_by_tag(spec) + nav = controls["next1"].callback.params + result = {tag: ctrl.value for tag, ctrl in controls.items()} + result.update(nav["field_displays"][1]) + result["eventnum"] = "2" + result["field_type"] = "renamed" + return result + + out, com = pop_editeventvals(deepcopy(eeg), gui=True, renderer=Renderer(), return_com=True) + assert out["event"][0]["type"] == eeg["event"][0]["type"] + assert out["event"][1]["type"] == "renamed" + assert "'changefield', {2 'type' 'renamed'}" in com + + +def test_qt_renderer_navigation_updates_eventnum_and_fields(): + """Clicking next1 advances eventnum and rewrites field_* widgets (#228).""" + os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + pytest.importorskip("PySide6") + from eegprep.functions.guifunc import qt as qt_module + + if qt_module.QtCore is None or qt_module.QtWidgets is None: + pytest.skip("PySide6 Qt libraries unavailable in this environment") + QtDialogRenderer = qt_module.QtDialogRenderer + + eeg = _epoched_eeg() + renderer = QtDialogRenderer() + spec = pop_editeventvals_dialog_spec(eeg) + _app, dialog, widgets = renderer.build_dialog(spec) + try: + assert widgets["eventnum"].text() == "1" + widgets["next1"].click() + assert widgets["eventnum"].text() == "2" + # field_type widget now reflects event #2's display value. + assert widgets["field_type"].text() == str(eeg["event"][1]["type"]) + # Going past the end clamps at the last event. + widgets["next10"].click() + assert widgets["eventnum"].text() == str(len(eeg["event"])) + widgets["back10"].click() + assert widgets["eventnum"].text() == "1" + finally: + dialog.close()