diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f039ed..77c0b37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,28 @@ # Changelog +## 0.7.1 + +Requires ObjectState 3 (`objectstate>=3.0,<4`) and `formbridge>=0.1.2,<0.2`. +No public name is removed. + +Changed: + +- An unset field's placeholder is one read: `WidgetService. + show_resolved_value_as_placeholder` shows `state.get_resolved_value(path)`. + The value-update path used to build its own context stack + (`build_context_stack(ancestor_objects=...)`, removed in ObjectState 3) and + read the object's attribute, and it used the default prefix instead of the + form's `placeholder_prefix`; the two refresh paths in `ParameterOpsService` + each repeated the resolve-and-format step. All three call the one method. +- Provenance navigation reads `state.get_provenance(path)` once per action + (`ProvenanceNavigationMixin._provenance_source`). When another config of the + same object supplies the value, the source state is the widget's own state. + ObjectState 3 reports that source as `(None, source_type)` for a state + without a scope id; the target path used to be looked up in the global + state for that scope. +- `AbstractManagerWidget` no longer builds a `LiveContextResolver` it never + called. + ## 0.7.0 Generic machinery moved here from three OpenHCS GUI classes. Needs diff --git a/pyproject.toml b/pyproject.toml index b28eb05..4a0a852 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,11 +25,11 @@ keywords = ["pyqt6", "forms", "reactive", "gui", "widgets", "dataclass"] dependencies = [ "annotated-types>=0.7.0", - "formbridge>=0.1.1,<0.2", + "formbridge>=0.1.2,<0.2", "magicgui>=0.7.0", "metaclass-registry>=0.1.5", "PyQt6>=6.4.0", - "objectstate>=2.0,<3", + "objectstate>=3.0,<4", "psutil>=5.9", "Pygments>=2.15", "python-introspect>=0.2.6,<0.3", diff --git a/requirements-ci.txt b/requirements-ci.txt index 8a7e5f5..8167663 100644 --- a/requirements-ci.txt +++ b/requirements-ci.txt @@ -1,2 +1,2 @@ # Source candidates needed to validate this checkout before coordinated publication. -# Empty: every first-party dependency is released (formbridge 0.1.1, objectstate 2.1.1). +# Empty: every first-party dependency is released (objectstate 3.0.0, formbridge 0.1.2). diff --git a/src/pyqt_reactive/__init__.py b/src/pyqt_reactive/__init__.py index 5507aec..059a5e1 100644 --- a/src/pyqt_reactive/__init__.py +++ b/src/pyqt_reactive/__init__.py @@ -19,7 +19,7 @@ - Cross-window reactive updates """ -__version__ = "0.7.0" +__version__ = "0.7.1" # Public API will be populated as modules are added __all__ = [ diff --git a/src/pyqt_reactive/services/parameter_ops_service.py b/src/pyqt_reactive/services/parameter_ops_service.py index 5337053..e723219 100644 --- a/src/pyqt_reactive/services/parameter_ops_service.py +++ b/src/pyqt_reactive/services/parameter_ops_service.py @@ -220,9 +220,6 @@ def refresh_single_placeholder(self, manager, field_name: str) -> None: if not PyQt6WidgetEnhancer.supports_placeholder(widget): return - # Compute full dotted path for nested PFMs - full_path = f"{manager.field_id}.{field_name}" if manager.field_id else field_name - # Only refresh if value is None (needs placeholder) # Use manager.parameters (scoped) not state.parameters (full paths) current_value = manager.parameters.get(field_name) @@ -231,26 +228,7 @@ def refresh_single_placeholder(self, manager, field_name: str) -> None: logger.debug(f"🔬 RESET_TRACE: value is not None, no placeholder needed") return - logger.debug(f"🔬 RESET_TRACE: value is None, calling get_resolved_value...") - - from objectstate import LazyDefaultPlaceholderService - - # Get raw resolved value from ObjectState (handles context building internally) - # Use full_path for nested PFMs - resolved_value = manager.state.get_resolved_value(full_path) - logger.debug(f"🔬 RESET_TRACE: resolved_value={repr(resolved_value)[:50]}") - - # Format for display (VIEW responsibility) - placeholder_text = LazyDefaultPlaceholderService._format_placeholder_text( - resolved_value, manager.config.placeholder_prefix - ) - logger.debug(f" 📝 Formatted placeholder: {repr(placeholder_text)[:50]}") - - if placeholder_text: - # Use type-safe method that passes actual value for checkbox groups - PyQt6WidgetEnhancer.apply_placeholder_with_value(widget, resolved_value, placeholder_text) - logger.debug(f" ✅ Applied placeholder to widget") - + if manager._widget_service.show_resolved_value_as_placeholder(widget, field_name, manager): # Keep enabled-field styling in sync when placeholder changes the visual state if field_name == 'enabled': try: @@ -321,7 +299,6 @@ def refresh_all_placeholders(self, manager) -> None: return from pyqt_reactive.forms.widget_strategies import PyQt6WidgetEnhancer - from objectstate import LazyDefaultPlaceholderService logger.debug(f"[PLACEHOLDER] {manager.field_id}: Refreshing placeholders via ObjectState") monitor = get_monitor("Placeholder resolution per field") @@ -339,18 +316,10 @@ def refresh_all_placeholders(self, manager) -> None: continue full_path = f"{manager.field_id}.{param_name}" if manager.field_id else param_name - current_value = manager.state.parameters.get(full_path) - should_apply_placeholder = current_value is None - resolved_value = manager.state.get_resolved_value(full_path) - - if should_apply_placeholder: + if manager.state.parameters.get(full_path) is None: with monitor.measure(): - # Format for display (VIEW responsibility) - placeholder_text = LazyDefaultPlaceholderService._format_placeholder_text( - resolved_value, manager.config.placeholder_prefix + manager._widget_service.show_resolved_value_as_placeholder( + widget, param_name, manager ) - if placeholder_text: - # Use type-safe method that passes actual value for checkbox groups - PyQt6WidgetEnhancer.apply_placeholder_with_value(widget, resolved_value, placeholder_text) elif PyQt6WidgetEnhancer.has_placeholder_state(widget): PyQt6WidgetEnhancer._clear_placeholder_state(widget) diff --git a/src/pyqt_reactive/services/widget_service.py b/src/pyqt_reactive/services/widget_service.py index 9137812..94aa0ca 100644 --- a/src/pyqt_reactive/services/widget_service.py +++ b/src/pyqt_reactive/services/widget_service.py @@ -302,36 +302,31 @@ def _apply_context_behavior( return if value is None: - logger.debug("Computing placeholder for None value") - from pyqt_reactive.forms.parameter_form_manager import ParameterFormManager - from objectstate import ObjectStateRegistry, build_context_stack - - # Get ancestor objects for context stack - ancestor_objects = ObjectStateRegistry.get_ancestor_objects(manager.scope_id or "") - - # Build context stack - stack = build_context_stack( - object_instance=manager.object_instance, - ancestor_objects=ancestor_objects, - ) - - with stack: - placeholder_text = manager.service.get_placeholder_text(param_name, type(manager.object_instance)) - logger.debug("Resolved placeholder text: %r", placeholder_text) - if placeholder_text: - # Get the resolved value directly for type-safe placeholder application - try: - resolved_value = getattr(manager.object_instance, param_name) - except AttributeError: - resolved_value = None - self.widget_enhancer.apply_placeholder_with_value(widget, resolved_value, placeholder_text) - else: - logger.debug("No placeholder text returned") + self.show_resolved_value_as_placeholder(widget, param_name, manager) elif value is not None: if self.widget_enhancer.has_placeholder_state(widget): logger.debug("Clearing placeholder state for concrete value") self.widget_enhancer._clear_placeholder_state(widget) + def show_resolved_value_as_placeholder(self, widget: QWidget, param_name: str, manager) -> bool: + """Preview an unset field: show the value it resolves to; return whether one was shown. + + The preview is the state's resolved read of the field. The field's own + value stays unset, and the source of the shown value is the same state's + ``get_provenance`` answer for the same path. + """ + from objectstate import LazyDefaultPlaceholderService + + full_path = f"{manager.field_id}.{param_name}" if manager.field_id else param_name + resolved_value = manager.state.get_resolved_value(full_path) + placeholder_text = LazyDefaultPlaceholderService._format_placeholder_text( + resolved_value, manager.config.placeholder_prefix + ) + if not placeholder_text: + return False + self.widget_enhancer.apply_placeholder_with_value(widget, resolved_value, placeholder_text) + return True + def clear_widget_to_default_state(self, widget: QWidget) -> None: """Clear widget to its default/empty state for reset operations.""" if isinstance(widget, QLineEdit): diff --git a/src/pyqt_reactive/widgets/shared/abstract_manager_widget.py b/src/pyqt_reactive/widgets/shared/abstract_manager_widget.py index 320b366..44009b2 100644 --- a/src/pyqt_reactive/widgets/shared/abstract_manager_widget.py +++ b/src/pyqt_reactive/widgets/shared/abstract_manager_widget.py @@ -42,7 +42,6 @@ FormattingConfig, ObjectStatePreviewFormattingService, ) -from objectstate import LiveContextResolver from pyqt_reactive.animation import FlashMixin from pyqt_reactive.widgets.shared.manager_ui_scaffold import setup_manager_widget_ui from formbridge.managers.item_hooks import ManagerItemHooks @@ -693,9 +692,6 @@ def __init__(self, service_adapter: UiHostServices, color_scheme=None, parent=No self.manager_header = None self.item_list: Optional[ReorderableListWidget] = None - # Live context resolver for config attribute resolution - self._live_context_resolver = LiveContextResolver() - # Per-update-cycle scope cache: item_id -> scope_id (cleared at start of each update) self._item_scope_cache: Dict[int, str] = {} self._pending_list_visual_scope_ids: dict[str, set[str]] = {} diff --git a/src/pyqt_reactive/widgets/shared/clickable_help_components.py b/src/pyqt_reactive/widgets/shared/clickable_help_components.py index 0384304..a2c2d33 100644 --- a/src/pyqt_reactive/widgets/shared/clickable_help_components.py +++ b/src/pyqt_reactive/widgets/shared/clickable_help_components.py @@ -6,7 +6,7 @@ from dataclasses import dataclass from typing import Optional, Union -from objectstate import ObjectState +from objectstate import ObjectState, ObjectStateRegistry from PyQt6.QtCore import QRect, QRectF, QSize, Qt, QTimer, pyqtSignal from PyQt6.QtGui import QColor, QCursor, QFont, QPainter from PyQt6.QtWidgets import ( @@ -521,31 +521,30 @@ def set_provenance_info(self, state, dotted_path: str) -> None: self._state = state self._dotted_path = dotted_path - def _has_provenance(self) -> bool: - """Check if this field has a provenance source (inherited from ancestor). + def _provenance_source(self) -> tuple[str | None, type] | None: + """The scope and type that supply this field's inherited value. - Returns True if the field inherits from an ancestor, even if that ancestor - is the root scope (GlobalPipelineConfig, scope_id=''). + ``None`` when the field is set on this field itself. The scope is ``''`` + for the global config and this state's own scope id (``None`` for a state + without one) when another config of the same object supplies the value. """ if not self._state or not self._dotted_path: - logger.debug(f"ProvenanceMixin._has_provenance: NO - state={self._state}, dotted_path={self._dotted_path}") - return False - try: - result = self._state.get_provenance(self._dotted_path) - logger.debug(f"ProvenanceMixin._has_provenance: result={result}, dotted_path={self._dotted_path}") - # Check if result is a valid tuple with a type class - # Empty scope_id ('') means GlobalPipelineConfig (root scope) - if not result or not isinstance(result, tuple) or len(result) != 2: - return False - scope_id, source_type = result - # Empty scope_id is valid (represents root/GlobalPipelineConfig) - # Just need a valid type class - return source_type is not None - except Exception as e: - logger.debug(f"ProvenanceMixin._has_provenance: EXCEPTION - {e}") + return None + return self._state.get_provenance(self._dotted_path) + + def _has_provenance(self) -> bool: + """Whether this field's value is inherited from another field.""" + return self._provenance_source() is not None + + def _navigate_to_provenance_source(self) -> bool: + """Go to the field that supplies this field's value; False when it is set here.""" + source = self._provenance_source() + if source is None: return False + self._navigate_to_source(*source) + return True - def _navigate_to_source(self, source_scope_id: str, source_type: type) -> None: + def _navigate_to_source(self, source_scope_id: str | None, source_type: type) -> None: """Open the source window (creating if needed) and scroll to the field. Args: @@ -553,28 +552,27 @@ def _navigate_to_source(self, source_scope_id: str, source_type: type) -> None: source_type: The TYPE that has the concrete value (may differ from local container type due to MRO inheritance) """ - # Extract field_name from our local dotted path - field_name = self._dotted_path.split('.')[-1] if '.' in self._dotted_path else self._dotted_path - - logger.debug(f"_navigate_to_source: local_path={self._dotted_path}, field_name={field_name}, " - f"source_scope_id={source_scope_id}, source_type={source_type.__name__}") + field_name = self._dotted_path.rsplit(".", 1)[-1] - # Compute the target path in the source window: - # We need to find what path source_type is at in the target window's ObjectState + # Another config of this same object supplies the value: the source state is + # this state, whether or not it is registered under a scope id. + same_window = ( + isinstance(self._state, ObjectState) and self._state.scope_id == source_scope_id + ) + source_state = ( + self._state if same_window else ObjectStateRegistry.get_by_scope(source_scope_id) + ) try: - target_path = self._compute_target_path(source_scope_id, source_type, field_name) + target_path = self._compute_target_path( + source_state, source_scope_id, source_type, field_name + ) except ProvenanceNavigationUnavailable as exc: logger.warning("Provenance navigation unavailable: %s", exc) return logger.info("Navigate: %s -> %s field %s", self._dotted_path, source_scope_id, target_path) - # Check if source is in the SAME window (sibling MRO inheritance within same scope) - if ( - isinstance(self._state, ObjectState) - and self._state.scope_id == source_scope_id - ): - logger.info(f"🔄 Same-window navigation: scrolling to {target_path}") + if same_window: self._scroll_within_current_window(target_path) return @@ -618,43 +616,34 @@ def _scroll_within_current_window(self, target_path: str) -> None: logger.warning(f"Could not find scrollable parent to navigate to {target_path}") - def _compute_target_path(self, source_scope_id: str, source_type: type, field_name: str) -> str: - """Compute the dotted path for source_type.field_name in the target window. - - The source_type might be at a different path in the target window than in our window. - For example: - - Local: WellFilterConfig at 'well_filter_config' - - Source: PathPlanningConfig at 'path_planning_config' - - We need to find what path source_type is at in the target window's ObjectState. - - IMPORTANT: If source_type is ui_hidden (not shown in the UI), we find the closest - visible subclass in the MRO that also has this field. For example: - - NapariDisplayConfig is ui_hidden=True - - NapariStreamingConfig inherits from NapariDisplayConfig and IS visible - - So we navigate to napari_streaming_config.field instead of napari_display_config.field + @staticmethod + def _compute_target_path( + source_state: ObjectState | None, + source_scope_id: str | None, + source_type: type, + field_name: str, + ) -> str: + """The dotted path of ``source_type.field_name`` in the source state's window. + + The source type can sit at another path there than in this window (local + ``well_filter_config``, source ``path_planning_config``). A source type that + is hidden in the UI answers with its closest visible subclass holding the + field, which ``project_ui_visible_field_path`` decides. """ - from objectstate import ObjectStateRegistry - - logger.debug(f"_compute_target_path: scope={source_scope_id}, source_type={source_type.__name__}, field={field_name}") - - # Get the target window's ObjectState - target_state = ObjectStateRegistry.get_by_scope(source_scope_id) - if target_state is None: + if source_state is None: raise ProvenanceNavigationUnavailable( f"no ObjectState registered for scope {source_scope_id!r}" ) - target_path = target_state.project_ui_visible_field_path(source_type, field_name) + target_path = source_state.project_ui_visible_field_path(source_type, field_name) if target_path is None: raise ProvenanceNavigationUnavailable( f"no visible UI path for type {source_type.__name__}.{field_name} " f"in scope {source_scope_id!r}" ) - - logger.info(f"Computed target path: {target_path} (source_type={source_type.__name__}, field={field_name})") return target_path + class ProvenanceLabel(QLabel, ProvenanceNavigationMixin): """QLabel that supports provenance navigation on hover/click. @@ -703,12 +692,11 @@ def leaveEvent(self, event) -> None: def mousePressEvent(self, event) -> None: """On click: navigate to provenance source window and field.""" - if event.button() == Qt.MouseButton.LeftButton and self._has_provenance(): - result = self._state.get_provenance(self._dotted_path) - if result is not None: - source_scope_id, source_type = result - self._navigate_to_source(source_scope_id, source_type) - return + if ( + event.button() == Qt.MouseButton.LeftButton + and self._navigate_to_provenance_source() + ): + return super().mousePressEvent(event) @@ -731,11 +719,7 @@ def __init__(self, text="^", color_scheme=None, parent=None): def _on_click(self): """Handle button click - navigate to provenance source.""" - if self._has_provenance(): - result = self._state.get_provenance(self._dotted_path) - if result: - source_scope_id, source_type = result - self._navigate_to_source(source_scope_id, source_type) + self._navigate_to_provenance_source() def enterEvent(self, event): """Show pointer cursor if field has provenance.""" diff --git a/tests/test_function_list_editor_tokens.py b/tests/test_function_list_editor_tokens.py index e0845cf..80f8cfb 100644 --- a/tests/test_function_list_editor_tokens.py +++ b/tests/test_function_list_editor_tokens.py @@ -29,9 +29,6 @@ def _reset_registry() -> None: ObjectStateRegistry._current_timeline = "main" ObjectStateRegistry._current_head = None ObjectStateRegistry._in_time_travel = False - ObjectStateRegistry._atomic_depth = 0 - ObjectStateRegistry._atomic_label = None - ObjectStateRegistry._atomic_triggering_scope = None @pytest.fixture(autouse=True) diff --git a/tests/test_preview_formatters.py b/tests/test_preview_formatters.py index fab0d63..53740f9 100644 --- a/tests/test_preview_formatters.py +++ b/tests/test_preview_formatters.py @@ -7,9 +7,9 @@ import pytest from objectstate.lazy_factory import ( FIELD_ABBREVIATIONS_REGISTRY, - GROUP_ABBREVIATIONS_REGISTRY, PREVIEW_LABEL_REGISTRY, LazyDataclassFactory, + abbreviation, ) from python_introspect import Enableable @@ -214,9 +214,8 @@ def format_request(request: PreviewFieldFormatRequest) -> str: assert segments == [("owned:5", "nested.value", None)] -def test_preview_groups_derive_identity_and_order_from_declared_types( - monkeypatch, -) -> None: +def test_preview_groups_derive_identity_and_order_from_declared_types() -> None: + @abbreviation("cfg") @dataclass class Config: first: int = 1 @@ -226,8 +225,6 @@ class Config: Config, "LazyPreviewGroupConfig", ) - monkeypatch.setitem(GROUP_ABBREVIATIONS_REGISTRY, Config, "cfg") - monkeypatch.setitem(GROUP_ABBREVIATIONS_REGISTRY, lazy_type, "copied") values = { "workers": 4, "config.first": 1, @@ -269,14 +266,12 @@ def type_for_path(self, field_path: str) -> type: ] -def test_preview_groups_keep_distinct_paths_with_the_same_declaration( - monkeypatch, -) -> None: +def test_preview_groups_keep_distinct_paths_with_the_same_declaration() -> None: + @abbreviation("cfg") @dataclass class Config: value: int = 1 - monkeypatch.setitem(GROUP_ABBREVIATIONS_REGISTRY, Config, "cfg") values = {"left.value": 1, "right.value": 2} class State: diff --git a/tests/test_provenance_navigation.py b/tests/test_provenance_navigation.py new file mode 100644 index 0000000..8bae04f --- /dev/null +++ b/tests/test_provenance_navigation.py @@ -0,0 +1,97 @@ +"""A label goes to the field that supplies its inherited value. + +The source is the state's ``get_provenance`` answer. When another config of the +same object supplies the value, the source state is the label's own state, +with or without a scope id. +""" + +from dataclasses import dataclass, field + +import objectstate.config as config_module +import pytest +from objectstate import ( + LazyDataclassFactory, + ObjectState, + ObjectStateRegistry, + mark_global_config_type, + set_base_config_type, + set_global_config_for_editing, +) + +from pyqt_reactive.services.scope_window_navigation import ScopeWindowNavigationService +from pyqt_reactive.widgets.shared.clickable_help_components import ProvenanceLabel + + +@dataclass +class SourceFilterConfig: + well_filter: int | None = None + + +@dataclass +class SourcePathConfig(SourceFilterConfig): + suffix: str | None = None + + +@dataclass +class SourceGlobalConfig: + filter_config: SourceFilterConfig = field(default_factory=SourceFilterConfig) + path_config: SourcePathConfig = field(default_factory=SourcePathConfig) + + +mark_global_config_type(SourceGlobalConfig) +LazySourceFilterConfig = LazyDataclassFactory.make_lazy_simple(SourceFilterConfig) +LazySourcePathConfig = LazyDataclassFactory.make_lazy_simple(SourcePathConfig) + +OWN = "path_config.well_filter" +SIBLING = "filter_config.well_filter" + + +@dataclass +class SourcePipeline: + filter_config: LazySourceFilterConfig = field(default_factory=LazySourceFilterConfig) + path_config: LazySourcePathConfig = field(default_factory=LazySourcePathConfig) + + +@pytest.fixture(params=[None, "plate"], ids=["no scope id", "scope id"]) +def label(qapp, request, monkeypatch): + previous_base = config_module._base_config_type + ObjectStateRegistry.clear() + set_base_config_type(SourceGlobalConfig) + set_global_config_for_editing( + SourceGlobalConfig, SourceGlobalConfig(SourceFilterConfig(well_filter=1)) + ) + state = ObjectState(SourcePipeline(), scope_id=request.param) + label = ProvenanceLabel("well filter", state=state, dotted_path=OWN) + label.scrolled_to = [] + label.window_requests = [] + monkeypatch.setattr(label, "_scroll_within_current_window", label.scrolled_to.append) + monkeypatch.setattr( + ScopeWindowNavigationService, + "navigate", + lambda request: label.window_requests.append(request) or request, + ) + try: + yield label + finally: + label.deleteLater() + ObjectStateRegistry.clear() + config_module._base_config_type = previous_base + + +def test_value_from_a_sibling_config_scrolls_this_window_to_the_sibling_field(label): + label._state.update_parameter(SIBLING, 7) + + assert label._state.get_resolved_value(OWN) == 7 + assert label._provenance_source() == (label._state.scope_id, SourceFilterConfig) + assert label._navigate_to_provenance_source() is True + assert label.scrolled_to == [SIBLING] + assert label.window_requests == [] + + +def test_value_set_on_the_field_has_no_source_to_go_to(label): + label._state.update_parameter(OWN, 9) + + assert label._has_provenance() is False + assert label._navigate_to_provenance_source() is False + assert label.scrolled_to == [] + assert label.window_requests == []