Merge pull request #620 from ZLLentz/fix_auto_scrollable

ENH/MNT/TST: make auto scrollable functional again

conda-recipe/meta.yaml

@@ -20,10 +20,8 @@ requirements:
   build:
   - python >=3.9
   - pip
+  - setuptools
   - setuptools_scm
-  host:
-  - python >=3.9
-  - pip
   run:
   - python >=3.9
   - coloredlogs

docs/source/upcoming_release_notes/620-fix_auto_scrollable.rst

@@ -0,0 +1,25 @@
+620 fix_auto_scrollable
+#######################
+
+API Breaks
+----------
+- N/A
+
+Features
+--------
+- N/A
+
+Bugfixes
+--------
+- Fix an issue where detailed tree screens would automatically load without
+  scrollbars. Now, the "auto" scrollbar setting is based primarily on the
+  apparent display type of the screen, not of the originally requested
+  display type, which may not be used if no such template exists.
+
+Maintenance
+-----------
+- N/A
+
+Contributors
+------------
+- zllentz

typhos/display.py

@@ -8,6 +8,7 @@
 import os
 import pathlib
 import webbrowser
+from pathlib import Path
 from typing import Dict, List, Optional, Union
 
 import ophyd
@@ -1032,15 +1033,15 @@ def __init__(
         embedded_templates: Optional[list[str]] = None,
         detailed_templates: Optional[list[str]] = None,
         engineering_templates: Optional[list[str]] = None,
-        display_type: Union[DisplayTypes, str, int] = 'detailed_screen',
-        scroll_option: Union[ScrollOptions, str, int] = 'auto',
+        display_type: Union[DisplayTypes, str, int] = 'embedded_screen',
+        scroll_option: Union[ScrollOptions, str, int] = ScrollOptions.auto,
         nested: bool = False,
     ):
         self._current_template = None
         self._forced_template = ''
         self._macros = {}
         self._display_widget = None
-        self._scroll_option = ScrollOptions.no_scroll
+        self._scroll_option = scroll_option
         self._searched = False
         self._hide_empty = False
         self._nested = nested
@@ -1113,11 +1114,9 @@ def hideEmpty(self, checked):
 
     @property
     def _layout_in_scroll_area(self) -> bool:
-        """Layout the widget in the scroll area or not, based on settings."""
+        """Layout the widget in the scroll area or not, based on settings and template."""
         if self.scroll_option == ScrollOptions.auto:
-            if self.display_type == DisplayTypes.embedded_screen:
-                return False
-            return True
+            return self.effective_display_type != DisplayTypes.embedded_screen
         elif self.scroll_option == ScrollOptions.scrollbar:
             return True
         elif self.scroll_option == ScrollOptions.no_scroll:
@@ -1241,6 +1240,28 @@ def display_type(self, value):
             self._display_type = value
             self.load_best_template()
 
+    @property
+    def effective_display_type(self) -> DisplayTypes:
+        """
+        Return the native display type of the current selected template.
+
+        It's possible to request e.g. an embedded screen or an engineering
+        screen and get the detailed screen or tree instead.
+
+        This returns the actual type of the screen that was chosen for use
+        in code that needs to respond to the actual template in use, rather
+        than the one requested.
+
+        If no template has been selected yet, this falls back to the
+        specified display type.
+        """
+        if self.current_template is None:
+            return self.display_type
+        try:
+            return get_template_display_type(self.current_template)
+        except ValueError:
+            return self.display_type
+
     @property
     def macros(self):
         """Get or set the macros for the display."""
@@ -1781,3 +1802,15 @@ def process(item, recursive=True):
         elif isinstance(widget, typhos_panel.TyphosSignalPanel):
             overall_status = bool(widget._panel_layout.visible_elements)
         widget.setVisible(overall_status)
+
+
+def get_template_display_type(template: Path) -> DisplayTypes:
+    """
+    Returns a template's native display type based on its name.
+
+    Raises a ValueError if the name cannot be determined.
+    """
+    # Either e.g. ClassName.detailed or core built-in like detailed_tree
+    template_name = template.stem
+    display_type = template_name.split(".")[-1].split("_")[0] + "_screen"
+    return normalize_display_type(display_type=display_type)

typhos/suite.py

@@ -257,7 +257,7 @@ def __init__(
         *,
         pin: bool = False,
         content_layout: QtWidgets.QLayout | None = None,
-        default_display_type: DisplayTypes = DisplayTypes.detailed_screen,
+        default_display_type: DisplayTypes = DisplayTypes.embedded_screen,
         scroll_option: ScrollOptions = ScrollOptions.auto,
     ):
         super().__init__(parent=parent)

typhos/tests/test_display.py

@@ -1,9 +1,14 @@
+from __future__ import annotations
+
+from pathlib import Path
+
 import ophyd
 import pytest
 from pydm import Display
 
 import typhos.display
 from typhos import utils
+from typhos.display import DisplayTypes, get_template_display_type
 
 from . import conftest
 from .conftest import show_widget
@@ -90,6 +95,7 @@ def test_display_without_md(motor, display):
 
 def test_display_with_md(motor, display):
     screen = 'engineering_screen.ui'
+    display.display_type = DisplayTypes.detailed_screen
     display.add_device(
         motor, macros={'detailed_screen': screen})
     display.load_best_template()
@@ -151,6 +157,7 @@ def test_display_device_name_property(motor, display, qtbot):
 def test_display_with_py_file(display, motor, qtbot):
     qtbot.add_widget(display)
     py_file = str(conftest.MODULE_PATH / 'utils' / 'display.py')
+    display.display_type = DisplayTypes.detailed_screen
     display.add_device(motor, macros={'detailed_screen': py_file})
     display.load_best_template()
     assert isinstance(display.display_widget, Display)
@@ -166,3 +173,46 @@ def test_display_with_sig_template(display, device, qapp, qtbot):
         device.setpoint.put(num)
         qapp.processEvents()
         assert display.display_widget.ui.setpoint.text() == str(num)
+
+
+@pytest.mark.parametrize(
+    "path, expected",
+    [
+        (Path("typhos/ui/core/detailed_screen.ui"), DisplayTypes.detailed_screen),
+        (Path("typhos/ui/core/detailed_tree.ui"), DisplayTypes.detailed_screen),
+        (Path("typhos/ui/core/embedded_screen.ui"), DisplayTypes.embedded_screen),
+        (Path("typhos/ui/core/engineering_screen.ui"), DisplayTypes.engineering_screen),
+        (Path("typhos/ui/devices/PositionerBase.detailed.ui"), DisplayTypes.detailed_screen),
+        (Path("typhos/ui/devices/PositionerBase.embedded.ui"), DisplayTypes.embedded_screen),
+        (Path("user/module/Potato.embedded.ui"), DisplayTypes.embedded_screen),
+        (Path("user/module/Potato.detailed.ui"), DisplayTypes.detailed_screen),
+        (Path("user/module/Potato.engineering.ui"), DisplayTypes.engineering_screen),
+    ]
+)
+def test_get_template_display_type_good(path: Path, expected: DisplayTypes):
+    assert get_template_display_type(path) == expected
+
+
+@pytest.mark.parametrize(
+    "path, expected",
+    [
+        (Path("user/module/enigma.ui"), ValueError),
+        (Path("user/module/not_very_detailed.ui"), ValueError),
+    ]
+)
+def test_get_template_display_type_bad(path: Path, expected: type[Exception]):
+    with pytest.raises(expected):
+        get_template_display_type(path)
+
+
+def test_display_effective_display_type(display, device, qapp, qtbot):
+    qtbot.add_widget(display)
+    assert display.effective_display_type == display.display_type
+    display.force_template = str(conftest.MODULE_PATH / 'utils' / 'sig.ui')
+    assert display.effective_display_type == display.display_type
+    display.force_template = str(conftest.MODULE_PATH.parent / 'ui' / 'core' / 'embedded_screen.ui')
+    assert display.effective_display_type == DisplayTypes.embedded_screen
+    display.force_template = str(conftest.MODULE_PATH.parent / 'ui' / 'core' / 'detailed_tree.ui')
+    assert display.effective_display_type == DisplayTypes.detailed_screen
+    display.force_template = str(conftest.MODULE_PATH.parent / 'ui' / 'core' / 'engineering_screen.ui')
+    assert display.effective_display_type == DisplayTypes.engineering_screen