Merge pull request pcdshub#563 from klauer/enh_oneline_positioner

ENH: layout changes for compact complex screens + row positioner widget

docs/source/index.rst

@@ -44,6 +44,7 @@ Related Projects
     python_methods.rst
     templates.rst
     release_notes.rst
+    upcoming_changes.rst
 
 .. toctree::
     :maxdepth: 3

docs/source/templates.rst

@@ -75,3 +75,20 @@ custom templates:
    point is expecting to find a ``str``, ``pathlib.Path``, or ``list`` of
    such objects at your entry point. One such example of how to do this can
    be found `here <https://github.com/pcdshub/pcdsdevices/blob/cab3fe158ebc0d032fe07f03ec52ca79cda90c6e/setup.py#L21>`_
+
+
+For top-level devices (e.g., ``at2l0``), the template load priority is as
+follows:
+
+- Happi-defined values (``"detailed_screen"``, ``embedded_screen"``,
+  ``"engineering_screen"``)
+- Device-specific screens, if available (named as ``ClassNameHere.detailed.ui``)
+- The detailed tree, if the device has sub-devices
+- The default templates
+
+For nested displays in a device tree, sub-device (e.g., ``at2l0.blade_01``)
+template load priority is as follows:
+
+- Device-specific screens, if available (named as ``ClassNameHere.embedded.ui``)
+- The detailed tree, if the device has sub-devices
+- The default templates

docs/source/upcoming_release_notes/563-row_positioner.rst

@@ -0,0 +1,50 @@
+563 row-positioner
+##################
+
+API Changes
+-----------
+- ``TyphosNoteEdit`` now supports ``.add_device()`` like other typhos widgets.
+  This is alongside its original ``setup_data`` API.
+- ``TyphosDeviceDisplay`` composite heuristics have been removed in favor of
+  simpler methods, described in the features section.
+
+Features
+--------
+- ``TyphosNoteEdit`` is now a ``TyphosBase`` object and is accessible in the Qt
+  designer.
+- Added new designable widget ``TyphosPositionerRowWidget``.  This compact
+  positioner widget makes dense motor-heavy screens much more space efficient.
+- The layout method for ``TyphosDeviceDisplay`` has changed.  For large device trees,
+  it now favors showing the compact "embedded" screens over detailed screens.  The order
+  of priority is now as follows:
+- For top-level devices (e.g., ``at2l0``), the template load priority is as follows:
+
+  * Happi-defined values (``"detailed_screen"``, ``embedded_screen"``, ``"engineering_screen"``)
+  * Device-specific screens, if available (named as ``ClassNameHere.detailed.ui``)
+  * The detailed tree, if the device has sub-devices
+  * The default templates
+
+- For nested displays in a device tree, sub-device (e.g., ``at2l0.blade_01``)
+  template load priority is as follows:
+
+  * Device-specific screens, if available (named as ``ClassNameHere.embedded.ui``)
+  * The detailed tree, if the device has sub-devices
+  * The default templates (``embedded_screen.ui``)
+
+Bugfixes
+--------
+- For devices which do not require keyword arguments to instantiate, the typhos
+  CLI will no longer require an empty dictionary.  That is, ``$ typhos
+  ophyd.sim.SynAxis[]`` is equivalent to ``$ typhos ophyd.sim.SynAxis[{}]``.
+  As before, ophyd's required "name" keyword argument is filled in by typhos by
+  default.
+
+
+Maintenance
+-----------
+- N/A
+
+Contributors
+------------
+- klauer
+- ZLLentz

pyproject.toml

@@ -37,7 +37,9 @@ TyphosDisplaySwitcherPlugin = "typhos.display:TyphosDisplaySwitcher"
 TyphosDisplayTitlePlugin = "typhos.display:TyphosDisplayTitle"
 TyphosHelpFramePlugin = "typhos.display:TyphosHelpFrame"
 TyphosMethodButtonPlugin = "typhos.func:TyphosMethodButton"
+TyphosNotesEditPlugin = "typhos.notes:TyphosNotesEdit"
 TyphosPositionerWidgetPlugin = "typhos.positioner:TyphosPositionerWidget"
+TyphosPositionerRowWidgetPlugin = "typhos.positioner:TyphosPositionerRowWidget"
 TyphosRelatedSuiteButtonPlugin = "typhos.related_display:TyphosRelatedSuiteButton"
 TyphosSignalPanelPlugin = "typhos.panel:TyphosSignalPanel"
 

typhos/alarm.py

@@ -329,7 +329,6 @@ def update_current_alarm(self):
                     f'Updated alarm from {self.alarm_summary} to {new_alarm} '
                     f'on alarm widget with channel {self.channels()[0]}'
                 )
-
         self.alarm_summary = new_alarm
 
     def set_alarm_color(self, alarm_level):

typhos/cli.py

@@ -17,14 +17,14 @@
 from pydm.widgets.template_repeater import FlowLayout
 from qtpy import QtCore, QtWidgets
 
-import typhos
-from typhos.app import get_qapp, launch_suite
-from typhos.benchmark.cases import run_benchmarks
-from typhos.benchmark.profile import profiler_context
-from typhos.display import DisplayTypes, ScrollOptions
-from typhos.suite import TyphosSuite
-from typhos.utils import (apply_standard_stylesheets, compose_stylesheets,
-                          nullcontext)
+from . import __version__ as typhos_version
+from . import utils
+from .app import get_qapp, launch_suite
+from .benchmark.cases import run_benchmarks
+from .benchmark.profile import profiler_context
+from .display import DisplayTypes, ScrollOptions
+from .suite import TyphosSuite
+from .utils import apply_standard_stylesheets, compose_stylesheets, nullcontext
 
 logger = logging.getLogger(__name__)
 
@@ -265,11 +265,11 @@ def _create_happi_client(cfg):
     """Create a happi client based on configuration ``cfg``."""
     import happi
 
-    import typhos.plugins.happi
+    from .plugins import happi as happi_plugin
 
-    if typhos.plugins.happi.HappiClientState.client:
+    if happi_plugin.HappiClientState.client:
         logger.debug("Using happi Client already registered with Typhos")
-        return typhos.plugins.happi.HappiClientState.client
+        return happi_plugin.HappiClientState.client
 
     logger.debug("Creating new happi Client from configuration")
     return happi.Client.from_config(cfg=cfg)
@@ -330,7 +330,7 @@ def create_suite(
         layout_obj = get_layout_from_cli(layout, cols)
         display_type_enum = get_display_type_from_cli(display_type)
         scroll_enum = get_scrollable_from_cli(scroll_option)
-        return typhos.TyphosSuite.from_devices(
+        return TyphosSuite.from_devices(
             devices,
             content_layout=layout_obj,
             default_display_type=display_type_enum,
@@ -453,7 +453,7 @@ def create_devices(device_names, cfg=None, fake_devices=False):
     devices = list()
 
     klass_regex = re.compile(
-        r'([a-zA-Z][a-zA-Z0-9\.\_]*)\[(\{.+})*[\,]*\]'  # noqa
+        r'([a-zA-Z][a-zA-Z0-9\.\_]*)\[(\{.*})*[\,]*\]'  # noqa
     )
 
     for device_name in device_names:
@@ -567,7 +567,7 @@ def typhos_run(
         The window created. This is returned after the application
         is done running. Primarily used in unit tests.
     """
-    with typhos.utils.no_device_lazy_load():
+    with utils.no_device_lazy_load():
         suite = create_suite(
             device_names,
             cfg=cfg,
@@ -616,7 +616,8 @@ def typhos_cli(args):
     args = parser.parse_args(args, TyphosArguments())
 
     if args.version:
-        print(f'Typhos: Version {typhos.__version__} from {typhos.__file__}')
+        typhos_file = sys.modules["typhos"].__file__
+        print(f'Typhos: Version {typhos_version} from {typhos_file}')
         return
 
     if any(

typhos/display.py

@@ -962,11 +962,6 @@ class TyphosDeviceDisplay(utils.TyphosBase, widgets.TyphosDesignerMixin,
         layout.
         If omitted, scroll_option is used instead.
 
-    composite_heuristics : bool, optional
-        Enable composite heuristics, which may change the suggested detailed
-        screen based on the contents of the added device.  See also
-        :meth:`.suggest_composite_screen`.
-
     embedded_templates : list, optional
         List of embedded templates to use in addition to those found on disk.
 
@@ -990,25 +985,20 @@ class TyphosDeviceDisplay(utils.TyphosBase, widgets.TyphosDesignerMixin,
     # Template types and defaults
     Q_ENUMS(_DisplayTypes)
     TemplateEnum = DisplayTypes  # For convenience
-
-    device_count_threshold = 0
-    signal_count_threshold = 30
     template_changed = QtCore.Signal(object)
 
     def __init__(
         self,
         parent: Optional[QtWidgets.QWidget] = None,
         *,
         scrollable: Optional[bool] = None,
-        composite_heuristics: bool = True,
         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',
         nested: bool = False,
     ):
-        self._composite_heuristics = composite_heuristics
         self._current_template = None
         self._forced_template = ''
         self._macros = {}
@@ -1021,6 +1011,12 @@ def __init__(
         self.templates = {name: [] for name in DisplayTypes.names}
         self._display_type = normalize_display_type(display_type)
 
+        if nested and self._display_type == DisplayTypes.detailed_screen:
+            # All nested displays should be embedded by default.
+            # Based on if they have subdevices, they may become detailed
+            # during the template loading process
+            self._display_type = DisplayTypes.embedded_screen
+
         instance_templates = {
             'embedded_screen': embedded_templates or [],
             'detailed_screen': detailed_templates or [],
@@ -1053,15 +1049,6 @@ def __init__(
             else:
                 self.scroll_option = ScrollOptions.no_scroll
 
-    @Property(bool)
-    def composite_heuristics(self):
-        """Allow composite screen to be suggested first by heuristics."""
-        return self._composite_heuristics
-
-    @composite_heuristics.setter
-    def composite_heuristics(self, composite_heuristics):
-        self._composite_heuristics = bool(composite_heuristics)
-
     @Property(_ScrollOptions)
     def scroll_option(self) -> ScrollOptions:
         """Place the display in a scrollable area."""
@@ -1087,27 +1074,31 @@ def hideEmpty(self, checked):
         if checked != self._hide_empty:
             self._hide_empty = checked
 
+    @property
+    def _layout_in_scroll_area(self) -> bool:
+        """Layout the widget in the scroll area or not, based on settings."""
+        if self.scroll_option == ScrollOptions.auto:
+            if self.display_type == DisplayTypes.embedded_screen:
+                return False
+            return True
+        elif self.scroll_option == ScrollOptions.scrollbar:
+            return True
+        elif self.scroll_option == ScrollOptions.no_scroll:
+            return False
+        return True
+
     def _move_display_to_layout(self, widget):
         if not widget:
             return
 
         widget.setParent(None)
-        if self.scroll_option == ScrollOptions.auto:
-            if self.display_type == DisplayTypes.embedded_screen:
-                scrollable = False
-            else:
-                scrollable = True
-        elif self.scroll_option == ScrollOptions.scrollbar:
-            scrollable = True
-        elif self.scroll_option == ScrollOptions.no_scroll:
-            scrollable = False
-        else:
-            scrollable = True
 
+        scrollable = self._layout_in_scroll_area
         if scrollable:
             self._scroll_area.setWidget(widget)
         else:
-            self.layout().addWidget(widget)
+            layout: QtWidgets.QVBoxLayout = self.layout()
+            layout.addWidget(widget, alignment=QtCore.Qt.AlignTop)
 
         self._scroll_area.setVisible(scrollable)
 
@@ -1292,12 +1283,12 @@ def size_hint(*args, **kwargs):
         self.template_changed.emit(template)
 
     def minimumSizeHint(self) -> QtCore.QSize:
-        if self._scroll_area is None:
-            return super().minimumSizeHint()
-        return QtCore.QSize(
-            self._scroll_area.viewportSizeHint().width(),
-            super().minimumSizeHint().height(),
-        )
+        if self._layout_in_scroll_area:
+            return QtCore.QSize(
+                self._scroll_area.viewportSizeHint().width(),
+                super().minimumSizeHint().height(),
+            )
+        return super().minimumSizeHint()
 
     @property
     def display_widget(self):
@@ -1447,19 +1438,20 @@ def search_for_templates(self):
                 logger.debug('Adding macro template %s: %s (total=%d)',
                              display_type, template, len(template_list))
 
-            # 2. Composite heuristics, if enabled
-            if self._composite_heuristics and view == 'detailed':
-                if self.suggest_composite_screen(cls):
-                    template_list.append(DETAILED_TREE_TEMPLATE)
-
-            # 3. Templates based on class hierarchy names
+            # 2. Templates based on class hierarchy names
             filenames = utils.find_templates_for_class(cls, view, paths)
             for filename in filenames:
                 if filename not in template_list:
                     template_list.append(filename)
                     logger.debug('Found new template %s: %s (total=%d)',
                                  display_type, filename, len(template_list))
 
+            # 3. Ensure that the detailed tree template makes its way in for
+            #    all top-level screens, if no class-specific screen exists
+            if DETAILED_TREE_TEMPLATE not in template_list:
+                if not self._nested or self.suggest_composite_screen(cls):
+                    template_list.append(DETAILED_TREE_TEMPLATE)
+
             # 4. Default templates
             template_list.extend(
                 [templ for templ in DEFAULT_TEMPLATES[display_type]
@@ -1476,31 +1468,10 @@ def suggest_composite_screen(cls, device_cls):
         composite : bool
             If True, favor the composite screen.
         """
-        num_devices = 0
-        num_signals = 0
-        for attr, component in utils._get_top_level_components(device_cls):
-            num_devices += issubclass(component.cls, ophyd.Device)
-            num_signals += issubclass(component.cls, ophyd.Signal)
-
-        specific_screens = cls._get_specific_screens(device_cls)
-        if (len(specific_screens) or
-                (num_devices <= cls.device_count_threshold and
-                 num_signals >= cls.signal_count_threshold)):
-            # 1. There's a custom screen - we probably should use them
-            # 2. There aren't many devices, so the composite display isn't
-            #    useful
-            # 3. There are many signals, which should be broken up somehow
-            composite = False
-        else:
-            # 1. No custom screen, or
-            # 2. Many devices or a relatively small number of signals
-            composite = True
-
-        logger.debug(
-            '%s screens=%s num_signals=%d num_devices=%d -> composite=%s',
-            device_cls, specific_screens, num_signals, num_devices, composite
-        )
-        return composite
+        for _, component in utils._get_top_level_components(device_cls):
+            if issubclass(component.cls, ophyd.Device):
+                return True
+        return False
 
     @classmethod
     def from_device(cls, device, template=None, macros=None, **kwargs):