Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: add --screenshot to the typhos CLI #566

Merged
merged 7 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions docs/source/upcoming_release_notes/566-screenshot.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
566 screenshot
#################

API Changes
-----------
- Added ``TyphosSuite.save_screenshot`` which takes a screenshot of the entire
suite as-displayed.
- Added ``TyphosSuite.save_device_screenshots`` which takes individual
screenshots of each device display in the suite and saves them to the
provided formatted filename.

Features
--------
- Add ``typhos --screenshot filename_pattern`` to take screenshots of typhos
displays prior to exiting early (in combination with ``--exit-after``).

Bugfixes
--------
- N/A

Maintenance
-----------
- N/A

Contributors
------------
- klauer
97 changes: 75 additions & 22 deletions typhos/cli.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
"""This module defines the ``typhos`` command line utility"""
from __future__ import annotations

import argparse
import ast
import inspect
import logging
import re
import signal
import sys
import types
from typing import Optional

import coloredlogs
Expand All @@ -25,6 +28,31 @@

logger = logging.getLogger(__name__)


class TyphosArguments(types.SimpleNamespace):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a big fan of using placeholder classes like this for arg type handling

"""Type hints for ``typhos`` CLI entrypoint arguments."""

devices: list[str]
layout: str
cols: int
display_type: str
scrollable: str
size: Optional[str]
hide_displays: bool
happi_cfg: Optional[str]
fake_device: bool
version: bool
verbose: bool
dark: bool
stylesheet_override: Optional[list[str]]
stylesheet_add: Optional[list[str]]
profile_modules: Optional[list[str]]
profile_output: Optional[str]
benchmark: Optional[list[str]]
exit_after: Optional[float]
screenshot_filename: Optional[str]


# Argument Parser Setup
parser = argparse.ArgumentParser(
description=(
Expand Down Expand Up @@ -181,6 +209,16 @@
"seconds"
),
)
parser.add_argument(
'--screenshot',
dest="screenshot_filename",
help=(
"Save screenshot(s) of all contained TyphosDeviceDisplay instances to "
"this filename pattern prior to exiting early. This name may contain "
"f-string style variables, including: suite_title, widget_title, "
"device, and name."
),
)


# Append to module docs
Expand Down Expand Up @@ -484,7 +522,8 @@ def typhos_run(
initial_size: Optional[str] = None,
show_displays: bool = True,
exit_after: Optional[float] = None,
) -> QtWidgets.QMainWindow:
screenshot_filename: Optional[str] = None,
) -> Optional[QtWidgets.QMainWindow]:
"""
Run the central typhos part of typhos.

Expand Down Expand Up @@ -516,6 +555,11 @@ def typhos_run(
show_displays : bool, optional
If True (default), open all the included device displays.
If False, do not open any of the displays.
screenshot_filename : str, optional
Save screenshot(s) of all contained TyphosDeviceDisplay instances to
this filename pattern prior to exiting early. This name may contain
f-string style variables, including: suite_title, widget_title,
device, and name.

Returns
-------
Expand All @@ -534,34 +578,42 @@ def typhos_run(
scroll_option=scroll_option,
show_displays=show_displays,
)
if suite:
if initial_size is not None:
try:
initial_size = QtCore.QSize(
*(int(opt) for opt in initial_size.split(','))
)
except TypeError as exc:
raise ValueError(
"Invalid --size argument. Expected a two-element pair "
"of comma-separated integers, e.g. --size 1000,1000"
) from exc

def exit_early():
logger.warning(
"Exiting typhos early due to --exit-after=%s CLI argument.",
exit_after

if suite is None:
logger.debug("Suite creation failure")
return None

if initial_size is not None:
try:
initial_size = QtCore.QSize(
*(int(opt) for opt in initial_size.split(','))
)
sys.exit(0)
except TypeError as exc:
raise ValueError(
"Invalid --size argument. Expected a two-element pair "
"of comma-separated integers, e.g. --size 1000,1000"
) from exc

def exit_early():
logger.warning(
"Exiting typhos early due to --exit-after=%s CLI argument.",
exit_after
)

if screenshot_filename is not None:
suite.save_device_screenshots(screenshot_filename)

sys.exit(0)

if exit_after is not None and exit_after >= 0:
QtCore.QTimer.singleShot(exit_after * 1000.0, exit_early)
if exit_after is not None and exit_after >= 0:
QtCore.QTimer.singleShot(exit_after * 1000.0, exit_early)

return launch_suite(suite, initial_size=initial_size)
return launch_suite(suite, initial_size=initial_size)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the cleanup/dedentation you did here



def typhos_cli(args):
"""Command Line Application for Typhos."""
args = parser.parse_args(args)
args = parser.parse_args(args, TyphosArguments())

if args.version:
print(f'Typhos: Version {typhos.__version__} from {typhos.__file__}')
Expand Down Expand Up @@ -602,6 +654,7 @@ def typhos_cli(args):
initial_size=args.size,
show_displays=not args.hide_displays,
exit_after=args.exit_after,
screenshot_filename=args.screenshot_filename,
)

return suite
Expand Down
3 changes: 3 additions & 0 deletions typhos/display.py
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,9 @@ def add_device(self, device, macros=None):
self.macros = self._build_macros_from_device(device, macros=macros)
self.load_best_template()

if not self.windowTitle():
self.setWindowTitle(getattr(device, "name", ""))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity: which edge case is this fixing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All widgets in Qt can have window titles, as you already know, in case they're displayed as-is in the window manager.

TyphosDeviceDisplay or its subclasses may choose to customize the window title but they don't currently. This says "if there's no title set, at least give it the device name"

Then to answer your actual question: this title is then used during the screenshot process, with output saying "Taking a screenshot of (suite title) (device display title)":
[2023-08-09 09:40:41] - INFO - Saving screenshot of 'Typhos Suite - EpicsMotor': 'EpicsMotor' to 'EpicsMotor.png'


def search_for_templates(self):
"""Search the filesystem for device-specific templates."""
device = self.device
Expand Down
59 changes: 58 additions & 1 deletion typhos/suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ def get_subdisplay(self, display):

Parameters
----------
display :str or Device
display : str or Device
Name of screen or device

Returns
Expand Down Expand Up @@ -767,6 +767,63 @@ def save(self):
# Add the template to the docstring
save.__doc__ += textwrap.indent('\n' + utils.saved_template, '\t\t')

def save_screenshot(
self,
filename: str,
) -> bool:
"""Save a screenshot of this widget to ``filename``."""

image = utils.take_widget_screenshot(self)
if image is None:
logger.warning("Failed to take screenshot")
return False

logger.info(
"Saving screenshot of suite titled '%s' to '%s'",
self.windowTitle(), filename,
)
image.save(filename)
return True

def save_device_screenshots(
self,
filename_format: str,
) -> dict[str, str]:
"""Save screenshot(s) of devices to ``filename_format``."""

filenames = {}
for device in self.devices:
display = self.get_subdisplay(device)

if hasattr(display, "to_image"):
image = display.to_image()
else:
# This is a fallback for if/when we don't have a TyphosDisplay
image = utils.take_widget_screenshot(display)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know how often this happens? I was under the impression most everything was a TyphosDeviceDisplay

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't ever happen in the current codebase. The truth is the following:

  1. I added utils.take_widget_screenshot first, added tests and had it all working
  2. Then I shifted to "it'd be better to save each device display"
  3. Then I recalled that I had already made to_image on TyphosDeviceDisplay (a few years ago perhaps?)
  4. Then I needed to justify the work from (1) so this else clause stayed in


suite_title = self.windowTitle()
widget_title = display.windowTitle()
if image is None:
logger.warning(
"Failed to take screenshot of device: %s in %s",
device.name, suite_title,
)
continue

filename = filename_format.format(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that the CLI --screenshot can be formatted with the following variables:
suite_title, widget_title, device, name (or any attribute thereof)

This should be better communicated in the CLI description

suite_title=suite_title,
widget_title=widget_title,
device=device,
name=device.name,
)
logger.info(
"Saving screenshot of '%s': '%s' to '%s'",
suite_title, widget_title, filename,
)
image.save(filename)
filenames[device.name] = filename
return filenames

def _get_sidebar(self, widget):
items = {}
for group in self.top_level_groups.values():
Expand Down
Loading
Loading