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 restore functionality to snapshot restore page #97

Merged
merged 8 commits into from
Nov 12, 2024
24 changes: 24 additions & 0 deletions docs/source/upcoming_release_notes/97-restore_functionality.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
97 restore functionality
#################

API Breaks
----------
- N/A

Features
--------
- add RestorePage table tooltips with PV name, status, and severity
- add working restore button and dialog to RestorePage

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

Maintenance
-----------
- use signals and slots to coordinate RestorePage table live data display status
- add second linac snapshot, and include in demo config

Contributors
------------
- shilorigins
2 changes: 1 addition & 1 deletion superscore/bin/demo.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,5 @@ def main(*args, db_path=None, **kwargs):
client.save(entry)
if isinstance(entry, (Snapshot, Setpoint, Readback)):
filled.append(entry)
with IOCFactory.from_entries(filled)(prefix=''):
with IOCFactory.from_entries(filled, client)(prefix=''):
ui_main(*args, client=client, **kwargs)
100 changes: 97 additions & 3 deletions superscore/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from pathlib import Path
from typing import List
from unittest.mock import MagicMock
from uuid import UUID

import pytest

Expand Down Expand Up @@ -642,6 +643,98 @@ def linac_data():
return all_col, all_snapshot


def comparison_linac_snapshot():
_, snapshot = linac_data()
snapshot.title = 'AD Comparison'
snapshot.description = ('A snapshot with different values and statuses to compare '
'to the "standard" snapshot')
snapshot.uuid = UUID("8e0b1916-912a-457e-8ff9-4478b8018cec")

lcls_nc_snapshot, facet_snapshot, lcls_sc_snapshot = snapshot.children
lcls_nc_snapshot.uuid = UUID("4e217631-a595-4cdd-b918-a10c54ff8e11")
facet_snapshot.uuid = UUID('ac7f4854-8d3f-4461-9ebf-2321d092657f')
lcls_sc_snapshot.uuid = UUID('8f9d7f91-bd13-4c0d-ac8c-26aa80c72df1')

in20_snapshot, li21_snapshot, bsy_snapshot = lcls_nc_snapshot.children
in20_snapshot.uuid = UUID('a55281fe-6c20-4ed0-9d73-342b2ec4d1f9')
li21_snapshot.uuid = UUID('a430d9d2-9acb-4c98-be75-d61b674c478f')
bsy_snapshot.uuid = UUID('3a2c72f1-3792-4dba-8133-bd295c222ade')

in10_snapshot, li10_snapshot = facet_snapshot.children
in10_snapshot.uuid = UUID('04c1cfbf-4f52-49af-a3f8-e637b7ac42c6')
li10_snapshot.uuid = UUID('90255db1-95d6-4b65-9105-b7f09c623354')

gunb_snapshot, l0b_snapshot, _ = lcls_sc_snapshot.children
gunb_snapshot.uuid = UUID('59767800-60bd-4d1f-85b3-c71731818a4c')
l0b_snapshot.uuid = UUID('41cd90c4-d6d4-44bd-a4e4-04ef5c3920f5')

lasr_in20_snapshot = in20_snapshot.children[0]
lasr_in20_snapshot.uuid = UUID('769c7df6-e807-407c-b2e3-5c94e09cc1a2')

vac_li21_snapshot = li21_snapshot.children[0]
vac_li21_snapshot.uuid = UUID('1fc13363-cb6f-48bd-a26f-4d76cc0755eb')

vac_bsy_snapshot = bsy_snapshot.children[0]
vac_bsy_snapshot.uuid = UUID('11efd7e3-48fb-4f23-a3b5-cc337af2aa1c')

lasr_in10_snapshot = in10_snapshot.children[0]
lasr_in10_snapshot.uuid = UUID('9fb395b0-a544-4166-9b03-3b839d315b6a')

vac_li10_snapshot = li10_snapshot.children[0]
vac_li10_snapshot.uuid = UUID('e6bea38f-0799-4771-9a16-814a40ab42ab')

vac_gunb_snapshot, mgnt_gunb_snapshot, lasr_gunb_snapshot = gunb_snapshot.children
vac_gunb_snapshot.uuid = UUID('8118dcc6-2c9e-4b38-869e-2f0c724de4a8')
mgnt_gunb_snapshot.uuid = UUID('2b18360a-d038-4c80-a3aa-2739cdde7247')
lasr_gunb_snapshot.uuid = UUID('b82da301-8f85-4f62-89c2-e9c16e2e767d')

vac_l0b_snapshot = l0b_snapshot.children[0]
vac_l0b_snapshot.uuid = UUID('c1d13a88-3dbc-4f40-860b-9f63c793232f')

lasr_in20_value = lasr_in20_snapshot.children[0]
lasr_in20_value.uuid = UUID('ef321662-f98e-4511-b9b0-6f2d8037c302')
lasr_in20_value.data = -1
lasr_in20_value.severity = Severity.MAJOR

vac_li21_setpoint, vac_li21_readback = vac_li21_snapshot.children
vac_li21_setpoint.uuid = UUID('e977f215-a7c9-4caf-8f91-d2783f3e4a88')
vac_li21_setpoint.data = 0.0
vac_li21_setpoint.severity = Severity.MINOR
vac_li21_readback.uuid = UUID('949a9837-95bd-4ca0-8dad-f478f57143dd')

vac_bsy_value = vac_bsy_snapshot.children[0]
vac_bsy_value.uuid = UUID('b976bac4-d68b-45b0-a519-e0307a60b052')
vac_bsy_value.data = "lasdjfjasldfj"

lasr_in10_value = lasr_in10_snapshot.children[0]
lasr_in10_value.uuid = UUID('21bf36a2-002c-49fe-a7c3-eade33d62dfd')
lasr_in10_value.data = 640.68

vac_li10_value = vac_li10_snapshot.children[0]
vac_li10_value.uuid = UUID('732cb745-482f-40a7-b83c-d7f2d4ed2305')
vac_li10_value.data = .27

vac_gunb_value1, vac_gunb_value2 = vac_gunb_snapshot.children
vac_gunb_value1.uuid = UUID('0e6c4d09-2a77-4ac2-b57a-fc9c049e9063')
vac_gunb_value2.uuid = UUID('d2a45d2b-bb7c-4ccb-a2e3-5e5a44c7dd30')
vac_gunb_value2.data = True

mgnt_gunb_value = mgnt_gunb_snapshot.children[0]
mgnt_gunb_value.uuid = UUID('61c7ac48-77eb-430c-a86b-52c1267f8ef0')

lasr_gunb_value1, lasr_gunb_value2 = lasr_gunb_snapshot.children
lasr_gunb_value1.uuid = UUID('4719d31c-62fc-490b-9729-7889f0b79df8')
lasr_gunb_value1.severity = Severity.INVALID
lasr_gunb_value2.uuid = UUID('bced6e63-f4f8-4ab5-9256-66a7da66b160')

vac_l0b_value = vac_l0b_snapshot.children[0]
vac_l0b_value.uuid = UUID('de169754-cafd-4f38-9f26-cf92039e75d8')
vac_l0b_value.data = -15
vac_l0b_value.severity = Severity.MINOR

return snapshot


@pytest.fixture(scope='function')
def linac_backend():
all_col, all_snapshot = linac_data()
Expand Down Expand Up @@ -812,8 +905,9 @@ def sample_client(
return client


@pytest.fixture(scope='module')
def linac_ioc():
@pytest.fixture
def linac_ioc(linac_backend):
_, snapshot = linac_data()
with IOCFactory.from_entries(snapshot.children)(prefix="SCORETEST:") as ioc:
client = Client(backend=linac_backend)
with IOCFactory.from_entries(snapshot.children, client)(prefix="SCORETEST:") as ioc:
yield ioc
2 changes: 1 addition & 1 deletion superscore/tests/demo.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ ca = true
pva = true

[demo]
fixtures = linac_data
fixtures = linac_data comparison_linac_snapshot
29 changes: 10 additions & 19 deletions superscore/tests/ioc/ioc_factory.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
from multiprocessing import Process
from typing import Iterable, Mapping, Union
from typing import Iterable, Mapping

from caproto.server import PVGroup, pvproperty
from caproto.server import run as run_ioc
from epicscorelibs.ca import dbr

from superscore.model import Entry, Nestable, Parameter, Readback, Setpoint
from superscore.client import Client
from superscore.model import Entry, Readback, Setpoint


class TempIOC(PVGroup):
Expand All @@ -32,36 +33,26 @@ class IOCFactory:
Generates TempIOC subclasses bound to a set of PVs.
"""
@staticmethod
def from_entries(entries: Iterable[Entry], **ioc_options) -> PVGroup:
def from_entries(entries: Iterable[Entry], client: Client, **ioc_options) -> PVGroup:
"""
Defines and instantiates a TempIOC subclass containing all PVs reachable
from entries.
"""
attrs = IOCFactory.prepare_attrs(entries)
attrs = IOCFactory.prepare_attrs(entries, client)
IOC = type("IOC", (TempIOC,), attrs)
return IOC

@staticmethod
def collect_pvs(entries: Iterable[Entry]) -> Iterable[Union[Parameter, Setpoint, Readback]]:
"""Returns a collection of all PVs reachable from entries"""
pvs = []
q = entries.copy()
while len(q) > 0:
entry = q.pop()
if isinstance(entry, Nestable):
q.extend(entry.children)
else:
pvs.append(entry)
return pvs

@staticmethod
def prepare_attrs(entries: Iterable[Entry]) -> Mapping[str, pvproperty]:
def prepare_attrs(entries: Iterable[Entry], client: Client) -> Mapping[str, pvproperty]:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I'm on board with requiring the IocFactory to also be passed a client, when all it's doing is gathering leaf entries. That means if we want to use this elsewhere, we need to construct a valid Client with a backend that holds the entries.

I know it's a bit repetitious, but here I think it could be justified

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't like it either. I don't mind re-implementing the gather method if we have to, but the entries being passed had UUID children that needed to be resolved. I can look into why the UUIDs were swapped in, but lazy loading feels like it's causing duplicate code in multiple places, and maybe we should revisit its implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed 100%. I think throughout the UI it's not unreasonable to expect the client to exist, and call its .fill() method or something similar. (Maybe we add a filled=True optional argument to Client.search() or something as well)

I think in the test suite unless elsewise specified it's ok to have all the Entry's be filled. I think if you ever use the Filestore backend you'll end up getting lazy Entry's

"""
Turns a collecton of PVs into a Mapping from attribute names to
caproto.pvproperties. The mapping is suitable for passing into a type()
call as the dict arg.
"""
pvs = IOCFactory.collect_pvs(entries)
pvs = []
for entry in entries:
leaves = client._gather_leaves(entry)
pvs.extend(leaves)
attrs = {}
for entry in pvs:
value = entry.data if isinstance(entry, (Setpoint, Readback)) else None
Expand Down
24 changes: 22 additions & 2 deletions superscore/tests/test_page.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Largely smoke tests for various pages"""

from unittest.mock import MagicMock
from unittest.mock import MagicMock, patch

import pytest
from pytestqt.qtbot import QtBot
Expand All @@ -14,7 +14,7 @@
from superscore.widgets.page.entry import (BaseParameterPage, CollectionPage,
ParameterPage, ReadbackPage,
SetpointPage, SnapshotPage)
from superscore.widgets.page.restore import RestorePage
from superscore.widgets.page.restore import RestoreDialog, RestorePage
from superscore.widgets.page.search import SearchPage


Expand Down Expand Up @@ -241,3 +241,23 @@ def test_restore_page_toggle_live(qtbot: QtBot, restore_page):

toggle_live_button.click()
qtbot.waitUntil(lambda: all((tableView.isColumnHidden(column) for column in live_columns)))


@patch('superscore.control_layers.core.ControlLayer.put')
def test_restore_dialog_restore(
put_mock,
mock_client: Client,
simple_snapshot: Snapshot,
):
dialog = RestoreDialog(mock_client, simple_snapshot)
dialog.restore()
assert put_mock.call_args.args == mock_client._gather_data(simple_snapshot)
shilorigins marked this conversation as resolved.
Show resolved Hide resolved


def test_restore_dialog_remove_pv(mock_client: Client, simple_snapshot: Snapshot):
dialog = RestoreDialog(mock_client, simple_snapshot)
assert dialog.tableWidget.rowCount() == len(simple_snapshot.children)
remove_button_column = 2
dialog.tableWidget.setCurrentCell(1, remove_button_column)
dialog.delete_row()
assert dialog.tableWidget.rowCount() == len(simple_snapshot.children) - 1
shilorigins marked this conversation as resolved.
Show resolved Hide resolved
48 changes: 48 additions & 0 deletions superscore/ui/restore_dialog.ui
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?xml version="1.0" encoding="UTF-8"?>
<ui version="4.0">
<class>Form</class>
<widget class="QWidget" name="Form">
<property name="geometry">
<rect>
<x>0</x>
<y>0</y>
<width>382</width>
<height>395</height>
</rect>
</property>
<property name="windowTitle">
<string>Form</string>
</property>
<layout class="QGridLayout" name="gridLayout">
<item row="0" column="0" colspan="2">
<widget class="QLabel" name="topLabel">
<property name="text">
<string>PVs to Restore</string>
</property>
<property name="alignment">
<set>Qt::AlignCenter</set>
</property>
</widget>
</item>
<item row="1" column="0" colspan="2">
<widget class="QTableWidget" name="tableWidget"/>
</item>
<item row="2" column="1">
<widget class="QPushButton" name="restoreButton">
<property name="text">
<string>Restore</string>
</property>
</widget>
</item>
<item row="2" column="0">
<widget class="QPushButton" name="cancelButton">
<property name="text">
<string>Cancel</string>
</property>
</widget>
</item>
</layout>
</widget>
<resources/>
<connections/>
</ui>
49 changes: 28 additions & 21 deletions superscore/ui/restore_page.ui
Original file line number Diff line number Diff line change
Expand Up @@ -22,27 +22,7 @@
<property name="frameShadow">
<enum>QFrame::Raised</enum>
</property>
<layout class="QGridLayout" name="gridLayout_2" columnstretch="0,0,0,0,0,0,0">
<item row="0" column="4">
<spacer name="headerSpacer">
<property name="orientation">
<enum>Qt::Horizontal</enum>
</property>
<property name="sizeHint" stdset="0">
<size>
<width>40</width>
<height>20</height>
</size>
</property>
</spacer>
</item>
<item row="0" column="5">
<widget class="LiveButton" name="compareLiveButton">
<property name="text">
<string>Compare to Live</string>
</property>
</widget>
</item>
<layout class="QGridLayout" name="gridLayout_2" columnstretch="0,0,0,0,0,0,0,0">
<item row="0" column="6">
<widget class="QPushButton" name="compareSnapshotButton">
<property name="text">
Expand Down Expand Up @@ -82,6 +62,33 @@
</item>
</layout>
</item>
<item row="0" column="5">
<widget class="LiveButton" name="compareLiveButton">
<property name="text">
<string>Compare to Live</string>
</property>
</widget>
</item>
<item row="0" column="4">
<spacer name="headerSpacer">
<property name="orientation">
<enum>Qt::Horizontal</enum>
</property>
<property name="sizeHint" stdset="0">
<size>
<width>40</width>
<height>20</height>
</size>
</property>
</spacer>
</item>
<item row="0" column="7">
<widget class="QPushButton" name="restoreButton">
<property name="text">
<string>Restore</string>
</property>
</widget>
</item>
</layout>
</widget>
</item>
Expand Down
Loading