From bc4adf2d75afef7f44b3de3bc6d9f1bf5db9bf36 Mon Sep 17 00:00:00 2001 From: Mark Wolfman Date: Wed, 22 May 2024 17:01:19 -0500 Subject: [PATCH 1/5] ENH: The set_many utility now works with Ophyd positioners as well as signals. --- pcdsdevices/tests/test_utils.py | 29 +++++- pcdsdevices/utils.py | 176 ++++++++++++++++++-------------- 2 files changed, 127 insertions(+), 78 deletions(-) diff --git a/pcdsdevices/tests/test_utils.py b/pcdsdevices/tests/test_utils.py index c3763237b88..f499c9090b6 100644 --- a/pcdsdevices/tests/test_utils.py +++ b/pcdsdevices/tests/test_utils.py @@ -5,13 +5,13 @@ import pytest from ophyd import Component as Cpt -from ophyd import Device, Signal +from ophyd import Device, Signal, PVPositionerDone from .. import utils from ..device import GroupDevice from ..utils import (move_subdevices_to_start, post_ophyds_to_elog, reorder_components, set_standard_ordering, - sort_components_by_kind, sort_components_by_name) + sort_components_by_kind, sort_components_by_name, set_many) try: import pty @@ -262,3 +262,28 @@ class ChessPieces(Device): get_order(ChessPieces) == ['queen', 'rook', 'king', 'bishop', 'knight', 'pawn'] ) + + +def test_set_many(): + """Checks that set_many sets multiple ophyd objects, and that it works + with signals as well as positioners. + + Positioners and signals need different keyword arguments to their + ``set()`` methods, so this implicitly tests that those are done + properly. + + """ + # Create a dummy positioner with an extra signal + class MyPositioner(PVPositionerDone): + setpoint = Cpt(Signal) + another_signal = Cpt(Signal) + + device = MyPositioner() + assert device.done.get() == 0 + # Set the positioner and the extra signal together + st = set_many({device: 5, device.another_signal: 7}, raise_on_set_failure=True) + st.wait(timeout=3) + # Check that all the signals were set properly + assert device.done.get() == 1 + assert device.setpoint.get() == 5 + assert device.another_signal.get() == 7 diff --git a/pcdsdevices/utils.py b/pcdsdevices/utils.py index 15acee1efe6..e4774f3c7ab 100644 --- a/pcdsdevices/utils.py +++ b/pcdsdevices/utils.py @@ -33,24 +33,24 @@ logger = logging.getLogger(__name__) -arrow_up = '\x1b[A' -arrow_down = '\x1b[B' -arrow_right = '\x1b[C' -arrow_left = '\x1b[D' -shift_arrow_up = '\x1b[1;2A' -shift_arrow_down = '\x1b[1;2B' -shift_arrow_right = '\x1b[1;2C' -shift_arrow_left = '\x1b[1;2D' -alt_arrow_up = '\x1b[1;3A' -alt_arrow_down = '\x1b[1;3B' -alt_arrow_right = '\x1b[1;3C' -alt_arrow_left = '\x1b[1;3D' -ctrl_arrow_up = '\x1b[1;5A' -ctrl_arrow_down = '\x1b[1;5B' -ctrl_arrow_right = '\x1b[1;5C' -ctrl_arrow_left = '\x1b[1;5D' -plus = '+' -minus = '-' +arrow_up = "\x1b[A" +arrow_down = "\x1b[B" +arrow_right = "\x1b[C" +arrow_left = "\x1b[D" +shift_arrow_up = "\x1b[1;2A" +shift_arrow_down = "\x1b[1;2B" +shift_arrow_right = "\x1b[1;2C" +shift_arrow_left = "\x1b[1;2D" +alt_arrow_up = "\x1b[1;3A" +alt_arrow_down = "\x1b[1;3B" +alt_arrow_right = "\x1b[1;3C" +alt_arrow_left = "\x1b[1;3D" +ctrl_arrow_up = "\x1b[1;5A" +ctrl_arrow_down = "\x1b[1;5B" +ctrl_arrow_right = "\x1b[1;5C" +ctrl_arrow_left = "\x1b[1;5D" +plus = "+" +minus = "-" def re_arg(kwarg_map): @@ -62,15 +62,20 @@ def re_arg(kwarg_map): def myfunc(*args, new_kwarg=, **kwargs): ... """ + def decorator(func): def wrapped(*args, **kwargs): new_kwargs = {} for k, v in kwargs.items(): if k in kwarg_map: - print(f"DEPRECATION WARNING: keyword argument '{k}' is no longer valid. Use '{kwarg_map[k]}' instead.") + print( + f"DEPRECATION WARNING: keyword argument '{k}' is no longer valid. Use '{kwarg_map[k]}' instead." + ) new_kwargs[kwarg_map.get(k, k)] = v return func(*args, **new_kwargs) + return wrapped + return decorator @@ -99,7 +104,7 @@ def get_input(): input : str """ if termios is None: - raise RuntimeError('Not supported on this platform') + raise RuntimeError("Not supported on this platform") # Save old terminal settings old_settings = termios.tcgetattr(sys.stdin) @@ -109,16 +114,16 @@ def get_input(): # Swap to cbreak mode to get raw inputs tty.setcbreak(sys.stdin.fileno()) # Poll for input. This is interruptable with ctrl+c - while (not is_input()): + while not is_input(): time.sleep(0.01) # Read the first character inp = sys.stdin.read(1) # Read more if we have a control sequence - if inp == '\x1b': + if inp == "\x1b": extra_inp = sys.stdin.read(2) inp += extra_inp # Read even more if we had a shift/alt/ctrl modifier - if extra_inp == '[1': + if extra_inp == "[1": inp += sys.stdin.read(3) finally: # Restore the terminal to normal input mode @@ -153,6 +158,7 @@ def convert_unit(value, unit, new_unit): global ureg if ureg is None: import pint + ureg = pint.UnitRegistry() expr = ureg.parse_expression(unit) @@ -175,18 +181,25 @@ def ipm_screen(dettype, prefix, prefix_ioc): The PV prefix associated with the IOC running the device. """ - if (dettype == 'IPIMB'): - executable = '/reg/g/pcds/controls/pycaqt/ipimb/ipimb' - elif (dettype == 'Wave8'): - executable = '/reg/g/pcds/pyps/apps/wave8/latest/wave8' + if dettype == "IPIMB": + executable = "/reg/g/pcds/controls/pycaqt/ipimb/ipimb" + elif dettype == "Wave8": + executable = "/reg/g/pcds/pyps/apps/wave8/latest/wave8" else: - raise ValueError('Unknown detector type') + raise ValueError("Unknown detector type") if shutil.which(executable) is None: raise OSError(f"{executable} is not on path, we cannot start the screen") - logger.info(f'Opening {dettype} screen for {prefix}...') - arglist = [executable, '--base', prefix, '--ioc', prefix_ioc, - '--evr', prefix+':TRIG'] + logger.info(f"Opening {dettype} screen for {prefix}...") + arglist = [ + executable, + "--base", + prefix, + "--ioc", + prefix_ioc, + "--evr", + prefix + ":TRIG", + ] _ = subprocess.Popen(arglist) @@ -252,7 +265,7 @@ def schedule(): timer.start() -def get_status_value(status_info, *keys, default_value='N/A'): +def get_status_value(status_info, *keys, default_value="N/A"): """ Get the value of a dictionary key. @@ -277,8 +290,15 @@ def get_status_value(status_info, *keys, default_value='N/A'): return default_value -def get_status_float(status_info, *keys, default_value='N/A', precision=4, - format='f', scale=1.0, include_plus_sign=False): +def get_status_float( + status_info, + *keys, + default_value="N/A", + precision=4, + format="f", + scale=1.0, + include_plus_sign=False, +): """ Get the value of a dictionary key. @@ -312,17 +332,16 @@ def get_status_float(status_info, *keys, default_value='N/A', precision=4, if isinstance(value, (int, float)): value = float(value) * scale if include_plus_sign: - fmt = '{:+.%d%s}' % (precision, format) + fmt = "{:+.%d%s}" % (precision, format) else: - fmt = '{:.%d%s}' % (precision, format) + fmt = "{:.%d%s}" % (precision, format) return fmt.format(value) return value except KeyError: return default_value -def format_status_table(status_info, row_to_key, column_to_key, - row_identifier='Index'): +def format_status_table(status_info, row_to_key, column_to_key, row_identifier="Index"): """ Create a PrettyTable based on status information. @@ -345,7 +364,7 @@ def format_status_table(status_info, row_to_key, column_to_key, table.field_names = [row_identifier] + list(column_to_key) for row_name, row_key in row_to_key.items(): row = [ - get_status_value(status_info, row_key, key, 'value') + get_status_value(status_info, row_key, key, "value") for key in column_to_key.values() ] table.add_row([str(row_name)] + row) @@ -353,7 +372,7 @@ def format_status_table(status_info, row_to_key, column_to_key, return table -def combine_status_info(obj, status_info, attrs, separator='\n= {attr} ='): +def combine_status_info(obj, status_info, attrs, separator="\n= {attr} ="): """ Combine status information from the given attributes. @@ -378,7 +397,7 @@ def combine_status_info(obj, status_info, attrs, separator='\n= {attr} ='): lines.append(separator.format(attr=attr, parent=obj, child=child)) lines.append(child.format_status_info(status_info[attr])) - return '\n'.join(lines) + return "\n".join(lines) def doc_format_decorator(**doc_fmts): @@ -390,9 +409,11 @@ def doc_format_decorator(**doc_fmts): update automatically when we decide to change the constant. """ + def inner_decorator(func): func.__doc__ = func.__doc__.format(**doc_fmts) return func + return inner_decorator @@ -496,7 +517,7 @@ def set_many( owner: ophyd.ophydobj.OphydObject | None = None, timeout: Number | None = None, settle_time: Number | None = None, - raise_on_set_failure: bool = False + raise_on_set_failure: bool = False, ) -> ophyd.status.StatusBase: """ Call ``set`` on all given signal-to-value pairs with a single Status @@ -527,15 +548,16 @@ def set_many( """ statuses = [] log = owner.log if owner is not None else logger + set_kw = dict(timeout=timeout, settle_time=settle_time) for signal, value in to_set.items(): + # Decide which keywords are suitable for this signal + params = inspect.signature(signal.set).parameters + _kw = {key: val for key, val in set_kw.items() if key in params.keys()} + # Set the actual signal value try: - st = signal.set( - value, timeout=timeout, settle_time=settle_time - ) + st = signal.set(value, **_kw) except Exception: - log.exception( - "Failed to set %s to %s", signal.name, value - ) + log.exception("Failed to set %s to %s", signal.name, value) if raise_on_set_failure: raise else: @@ -552,9 +574,7 @@ def set_many( return status -def maybe_make_method( - func: Callable | None, owner: object -) -> Callable | None: +def maybe_make_method(func: Callable | None, owner: object) -> Callable | None: """ Bind ``func`` as a method of ``owner`` if ``self`` is the first parameter. @@ -623,34 +643,37 @@ def format_ophyds_to_html(obj, allow_child=False): return content # HelpfulNamespaces tend to lack names, maybe they won't some day - parent_default = ('Ophyd status: ' + - ', '.join('[...]' if isinstance(o, Iterable) - else o.name for o in obj)) - parent_name = getattr(obj, '__name__', parent_default[:60] + ' ...') + parent_default = "Ophyd status: " + ", ".join( + "[...]" if isinstance(o, Iterable) else o.name for o in obj + ) + parent_name = getattr(obj, "__name__", parent_default[:60] + " ...") # Wrap in a parent div out = ( - "
" + - content + - "
" + "
" + + content + + "
" ) return out # check if parent level ophyd object - elif (callable(getattr(obj, 'status', None)) and - ((getattr(obj, 'parent', None) is None and - getattr(obj, 'biological_parent', None) is None) or - allow_child)): + elif callable(getattr(obj, "status", None)) and ( + ( + getattr(obj, "parent", None) is None + and getattr(obj, "biological_parent", None) is None + ) + or allow_child + ): content = "" try: content = ( - f"" + - f"
{obj.status()}
" + f"" + + f"
{obj.status()}
" ) except Exception as ex: - logger.info(f'skipped {str(obj)}, due to Exception: {ex}') + logger.info(f"skipped {str(obj)}, due to Exception: {ex}") return content @@ -695,12 +718,13 @@ def post_ophyds_to_elog(objs, allow_child=False, hutch_elog=None): if hutch_elog is None: try: from elog.utils import get_primary_elog + hutch_elog = get_primary_elog() except ValueError: - logger.info('elog module exists, but no elog registered') + logger.info("elog module exists, but no elog registered") return else: - logger.info('Posting to provided elog') + logger.info("Posting to provided elog") post = format_ophyds_to_html(objs, allow_child=allow_child) @@ -711,8 +735,7 @@ def post_ophyds_to_elog(objs, allow_child=False, hutch_elog=None): # wrap post in head and tail final_post = collapse_list_head + post + collapse_list_tail - hutch_elog.post(final_post, tags=['ophyd_status'], - title='ophyd status report') + hutch_elog.post(final_post, tags=["ophyd_status"], title="ophyd status report") def reorder_components( @@ -743,6 +766,7 @@ def reorder_components( When used as a decorator with the reverse argument, this will return a function as required by the decorator interface. """ + # Special decorator handling def inner(cls: type[Device]) -> type[Device]: start_norm = _normalize_reorder_list(cls, start_with) @@ -777,16 +801,14 @@ def _normalize_reorder_list( output.append(reverse_map[obj]) except KeyError as exc: raise ValueError( - f'Received component {obj}, which is not from the device ' - f'class {cls}. We have components with the following ' + f"Received component {obj}, which is not from the device " + f"class {cls}. We have components with the following " f'names: {", ".join(cls._sig_attrs)}' ) from exc elif isinstance(obj, str): output.append(obj) else: - raise TypeError( - f'Received object {obj}, which is not a str or Component.' - ) + raise TypeError(f"Received object {obj}, which is not a str or Component.") return output @@ -818,6 +840,7 @@ def move_subdevices_to_start( When used as a decorator with the subdevice_cls argument, this will return a function as required by the decorator interface. """ + # Special decorator handling def inner(cls: type[Device]) -> type[Device]: device_names = [] @@ -859,6 +882,7 @@ def sort_components_by_name( When used as a decorator with the reverse argument, this will return a function as required by the decorator interface. """ + # Special decorator handling def inner(cls: type[Device]) -> type[Device]: alphabetical = list(sorted(cls._sig_attrs, reverse=reverse)) From 2ee94db12ac3a8e07a654a4c8625c8fd7af768a9 Mon Sep 17 00:00:00 2001 From: Mark Wolfman Date: Fri, 24 May 2024 22:28:15 -0500 Subject: [PATCH 2/5] Changed set_many to use an isinstance check instead of signature inspection. --- pcdsdevices/tests/test_utils.py | 6 +++--- pcdsdevices/utils.py | 12 +++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pcdsdevices/tests/test_utils.py b/pcdsdevices/tests/test_utils.py index f499c9090b6..4b79ce1b4e6 100644 --- a/pcdsdevices/tests/test_utils.py +++ b/pcdsdevices/tests/test_utils.py @@ -5,13 +5,13 @@ import pytest from ophyd import Component as Cpt -from ophyd import Device, Signal, PVPositionerDone +from ophyd import Device, PVPositionerDone, Signal from .. import utils from ..device import GroupDevice from ..utils import (move_subdevices_to_start, post_ophyds_to_elog, - reorder_components, set_standard_ordering, - sort_components_by_kind, sort_components_by_name, set_many) + reorder_components, set_many, set_standard_ordering, + sort_components_by_kind, sort_components_by_name) try: import pty diff --git a/pcdsdevices/utils.py b/pcdsdevices/utils.py index e4774f3c7ab..f8d95db3526 100644 --- a/pcdsdevices/utils.py +++ b/pcdsdevices/utils.py @@ -20,6 +20,7 @@ from ophyd.device import Component as Cpt from ophyd.device import Device from ophyd.ophydobj import Kind +from ophyd.positioner import PositionerBase from ._html import collapse_list_head, collapse_list_tail from .type_hints import Number, OphydDataType @@ -548,14 +549,15 @@ def set_many( """ statuses = [] log = owner.log if owner is not None else logger - set_kw = dict(timeout=timeout, settle_time=settle_time) for signal, value in to_set.items(): - # Decide which keywords are suitable for this signal - params = inspect.signature(signal.set).parameters - _kw = {key: val for key, val in set_kw.items() if key in params.keys()} # Set the actual signal value + if isinstance(signal, PositionerBase): + # Positioners don't have a *settle_time* parameter + kw = dict(timeout=timeout) + else: + kw = dict(timeout=timeout, settle_time=settle_time) try: - st = signal.set(value, **_kw) + st = signal.set(value, **kw) except Exception: log.exception("Failed to set %s to %s", signal.name, value) if raise_on_set_failure: From 7da6723546995e3a8ccdc95a4ee3c0ac9f78ec77 Mon Sep 17 00:00:00 2001 From: Mark Wolfman Date: Fri, 24 May 2024 22:43:03 -0500 Subject: [PATCH 3/5] =?UTF-8?q?Changed=20``set=5Fmany``=20to=20use=20try?= =?UTF-8?q?=E2=80=A6except=20for=20``Positioner``=20objects.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pcdsdevices/utils.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pcdsdevices/utils.py b/pcdsdevices/utils.py index f8d95db3526..f5333470e49 100644 --- a/pcdsdevices/utils.py +++ b/pcdsdevices/utils.py @@ -551,13 +551,12 @@ def set_many( log = owner.log if owner is not None else logger for signal, value in to_set.items(): # Set the actual signal value - if isinstance(signal, PositionerBase): - # Positioners don't have a *settle_time* parameter - kw = dict(timeout=timeout) - else: - kw = dict(timeout=timeout, settle_time=settle_time) try: - st = signal.set(value, **kw) + try: + st = signal.set(value, timeout=timeout, settle_time=settle_time) + except TypeError: + # It's probably a Positioner, which doesn't accept *settle_time* + st = signal.set(value, timeout=timeout) except Exception: log.exception("Failed to set %s to %s", signal.name, value) if raise_on_set_failure: From 9efb2e6fe2e8ac1a0ddaabda99f95573363ab233 Mon Sep 17 00:00:00 2001 From: Mark Wolfman Date: Fri, 24 May 2024 23:01:11 -0500 Subject: [PATCH 4/5] Added an explicit ``set_many()`` check that the exception is related. In ``set_many()`` we catch a TypeError to see if the *settle_time* parameter is accepted by each signal's ``set()`` method, and if it is we try again without the *settle_time* argument. If a TypeError is raised for another reason, this would also retry the ``set()`` call. We can't guarantee that this is indempotent. Now we explicity check that the TypeError is at least related to the *settle_time* argument before retrying. --- pcdsdevices/utils.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pcdsdevices/utils.py b/pcdsdevices/utils.py index f5333470e49..8314ee4b0be 100644 --- a/pcdsdevices/utils.py +++ b/pcdsdevices/utils.py @@ -554,9 +554,12 @@ def set_many( try: try: st = signal.set(value, timeout=timeout, settle_time=settle_time) - except TypeError: - # It's probably a Positioner, which doesn't accept *settle_time* - st = signal.set(value, timeout=timeout) + except TypeError as exc: + if "settle_time" in str(exc): + # It's probably a Positioner, which doesn't accept *settle_time* + st = signal.set(value, timeout=timeout) + else: + raise except Exception: log.exception("Failed to set %s to %s", signal.name, value) if raise_on_set_failure: From e70fddc9e2319cee331c3218963a26b37910edb0 Mon Sep 17 00:00:00 2001 From: Mark Wolfman Date: Tue, 28 May 2024 10:51:43 -0500 Subject: [PATCH 5/5] MNT: Removed unused import. --- pcdsdevices/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pcdsdevices/utils.py b/pcdsdevices/utils.py index 8314ee4b0be..acb4c0598f6 100644 --- a/pcdsdevices/utils.py +++ b/pcdsdevices/utils.py @@ -20,7 +20,6 @@ from ophyd.device import Component as Cpt from ophyd.device import Device from ophyd.ophydobj import Kind -from ophyd.positioner import PositionerBase from ._html import collapse_list_head, collapse_list_tail from .type_hints import Number, OphydDataType