Skip to content

Commit

Permalink
Update to use new attribute API and pyright
Browse files Browse the repository at this point in the history
  • Loading branch information
jsouter committed Dec 9, 2024
1 parent 3bdfc71 commit 3f39706
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 44 deletions.
2 changes: 1 addition & 1 deletion .copier-answers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ github_org: DiamondLightSource
package_name: fastcs_eiger
pypi: true
repo_name: fastcs-eiger
type_checker: mypy
type_checker: pyright
13 changes: 7 additions & 6 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ classifiers = [
description = "Eiger control system integration with FastCS"
dependencies = [
"aiohttp",
"fastcs~=0.7.0",
"fastcs @ git+https://github.com/DiamondLightSource/FastCS.git@main",
"numpy",
"pillow",
"typer",
Expand All @@ -27,11 +27,11 @@ dev = [
"tickit-devices @ git+https://github.com/dls-controls/tickit-devices.git@eiger-stream2",
"black",
"copier",
"mypy",
"myst-parser",
"pipdeptree",
"pre-commit",
"pydata-sphinx-theme>=0.12",
"pyright",
"pytest",
"pytest-asyncio",
"pytest-cov",
Expand All @@ -57,8 +57,9 @@ name = "Gary Yendell"
[tool.setuptools_scm]
version_file = "src/fastcs_eiger/_version.py"

[tool.mypy]
ignore_missing_imports = true # Ignore missing stubs in imported modules
[tool.pyright]
typeCheckingMode = "standard"
reportMissingImports = false # Ignore missing stubs in imported modules

[tool.pytest.ini_options]
# Run pytest with all our checkers, and don't spam us with massive tracebacks on error
Expand Down Expand Up @@ -91,12 +92,12 @@ passenv = *
allowlist_externals =
pytest
pre-commit
mypy
pyright
sphinx-build
sphinx-autobuild
commands =
pre-commit: pre-commit run --all-files --show-diff-on-failure {posargs}
type-checking: mypy src tests {posargs}
type-checking: pyright src tests {posargs}
tests: pytest --cov=fastcs_eiger --cov-report term --cov-report xml:cov.xml {posargs}
docs: sphinx-{posargs:build -EW --keep-going} -T docs build/html
"""
Expand Down
30 changes: 15 additions & 15 deletions src/fastcs_eiger/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
from typing import Optional

import typer
from fastcs.backends.asyncio_backend import AsyncioBackend
from fastcs.backends.epics.backend import EpicsBackend
from fastcs.backends.epics.gui import EpicsGUIOptions
from fastcs.launch import FastCS
from fastcs.transport.epics.options import (
EpicsGUIOptions,
EpicsIOCOptions,
EpicsOptions,
)

from fastcs_eiger import __version__
from fastcs_eiger.eiger_controller import EigerController
Expand Down Expand Up @@ -52,19 +55,16 @@ def ioc(

controller = EigerController(ip, port)

backend = EpicsBackend(controller, pv_prefix)
backend.create_gui(
EpicsGUIOptions(output_path=ui_path / "eiger.bob", title=f"Eiger - {pv_prefix}")
options = EpicsOptions(

Check warning on line 58 in src/fastcs_eiger/__main__.py

View check run for this annotation

Codecov / codecov/patch

src/fastcs_eiger/__main__.py#L58

Added line #L58 was not covered by tests
ioc=EpicsIOCOptions(pv_prefix=pv_prefix),
gui=EpicsGUIOptions(
output_path=ui_path / "eiger.bob", title=f"Eiger - {pv_prefix}"
),
)
backend.run()


@app.command()
def asyncio(ip: str = EigerIp, port: int = EigerPort):
controller = EigerController(ip, port)

backend = AsyncioBackend(controller)
backend.run_interactive_session()
launcher = FastCS(controller, options)
launcher.create_docs()
launcher.create_gui()
launcher.run()

Check warning on line 67 in src/fastcs_eiger/__main__.py

View check run for this annotation

Codecov / codecov/patch

src/fastcs_eiger/__main__.py#L64-L67

Added lines #L64 - L67 were not covered by tests


# test with: python -m fastcs_eiger
Expand Down
43 changes: 28 additions & 15 deletions src/fastcs_eiger/eiger_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
from typing import Any, Literal

import numpy as np
from fastcs.attributes import Attribute, AttrR, AttrRW, AttrW
from fastcs.controller import Controller, SubController
from fastcs.attributes import Attribute, AttrR, AttrRW, AttrW, Handler
from fastcs.controller import BaseController, Controller, SubController
from fastcs.datatypes import Bool, Float, Int, String
from fastcs.wrappers import command, scan
from PIL import Image
Expand Down Expand Up @@ -44,6 +44,9 @@
}


_datatypes = {int: Int(), float: Float(), str: String(), bool: Bool()}


def command_uri(key: str) -> str:
return f"detector/api/1.8.0/command/{key}"

Expand All @@ -62,10 +65,10 @@ class EigerHandler:
"""

uri: str
update_period: float = 0.2
update_period: float | None = 0.2

async def put(
self, controller: "EigerSubsystemController", _: AttrW, value: Any
self, controller: "EigerSubsystemController", attr: AttrW, value: Any
) -> None:
parameters_to_update = await controller.connection.put(self.uri, value)
if not parameters_to_update:
Expand All @@ -86,7 +89,7 @@ async def put(

await controller.queue_update(parameters_to_update)

async def update(self, controller: "EigerController", attr: AttrR) -> None:
async def update(self, controller: "EigerSubsystemController", attr: AttrR) -> None:
try:
response = await controller.connection.get(self.uri)
await attr.set(response["value"])
Expand Down Expand Up @@ -116,15 +119,16 @@ async def config_update(


@dataclass
class LogicHandler:
class LogicHandler(Handler):
"""
Handler for FastCS Attribute Creation
Dataclass that is called using the AttrR, AttrRW function.
Used for dynamically created attributes that are added for additional logic
"""

async def put(self, _: "EigerController", attr: AttrW, value: Any) -> None:
async def put(self, controller: BaseController, attr: AttrW, value: Any) -> None:
assert isinstance(attr, AttrR) # AttrW does not implement set

Check warning on line 131 in src/fastcs_eiger/eiger_controller.py

View check run for this annotation

Codecov / codecov/patch

src/fastcs_eiger/eiger_controller.py#L131

Added line #L131 was not covered by tests
await attr.set(value)


Expand Down Expand Up @@ -227,13 +231,21 @@ async def initialise(self) -> None:
print("\nAn HTTP request failed while introspecting detector:\n")
raise

def get_subsystem_controllers(self) -> list["EigerSubsystemController"]:
return [
controller
for controller in self.get_sub_controllers().values()
if isinstance(controller, EigerSubsystemController)
]

@scan(0.1)
async def update(self):
"""Periodically check for parameters that need updating from the detector."""
subsystem_controllers = self.get_subsystem_controllers()
await self.stale_parameters.set(
any(c.stale_parameters.get() for c in self.get_sub_controllers().values())
any(c.stale_parameters.get() for c in subsystem_controllers)
)
controller_updates = [c.update() for c in self.get_sub_controllers().values()]
controller_updates = [c.update() for c in subsystem_controllers]
await asyncio.gather(*controller_updates)


Expand Down Expand Up @@ -283,7 +295,7 @@ async def initialise(self) -> None:
attributes = self._create_attributes(parameters)

for name, attribute in attributes.items():
setattr(self, name, attribute)
self.attributes[name] = attribute

@classmethod
def _group(cls, parameter: EigerParameter):
Expand All @@ -305,15 +317,16 @@ def _create_attributes(cls, parameters: list[EigerParameter]):
for parameter in parameters:
match parameter.response["value_type"]:
case "float":
datatype = Float()
datatype = _datatypes[float]
case "int" | "uint":
datatype = Int()
datatype = _datatypes[int]
case "bool":
datatype = Bool()
datatype = _datatypes[bool]
case "string" | "datetime" | "State" | "string[]":
datatype = String()
datatype = _datatypes[str]
case _:
print(f"Failed to handle {parameter}")
continue

group = cls._group(parameter)
match parameter.response["access_mode"]:
Expand Down Expand Up @@ -365,7 +378,7 @@ async def update(self):
match getattr(self, attr_name, None):
# TODO: mypy doesn't understand AttrR as a type for some reason:
# `error: Expected type in class pattern; found "Any" [misc]`
case AttrR(updater=EigerConfigHandler() as updater) as attr: # type: ignore [misc]
case AttrR(updater=EigerConfigHandler() as updater) as attr:
parameter_updates.append(updater.config_update(self, attr))
case _ as attr:
print(f"Failed to handle update for {key}: {attr}")
Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def sim_eiger_controller(request):
)

# Wait until ready
while True:
while proc.stdout is not None:
line = proc.stdout.readline()
if "Starting HTTP server..." in line:
break
Expand Down
12 changes: 8 additions & 4 deletions tests/system/test_introspection.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from typing import Any

import pytest
from fastcs.attributes import Attribute, AttrR
from fastcs.attributes import Attribute, AttrR, AttrRW
from fastcs.datatypes import Float

from fastcs_eiger.eiger_controller import (
Expand All @@ -15,6 +15,7 @@
EigerMonitorController,
EigerParameter,
EigerStreamController,
EigerSubsystemController,
)

HERE = Path(__file__).parent
Expand Down Expand Up @@ -43,6 +44,7 @@ async def test_attribute_creation(sim_eiger_controller: EigerController):
serialised_parameters: dict[str, dict[str, Any]] = {}
subsystem_parameters = {}
for subsystem_name, subcontroller in controller.get_sub_controllers().items():
assert isinstance(subcontroller, EigerSubsystemController)
serialised_parameters[subsystem_name] = {}
subsystem_parameters[
subsystem_name
Expand Down Expand Up @@ -95,6 +97,7 @@ async def test_controller_groups_and_parameters(sim_eiger_controller: EigerContr

for subsystem in MISSING_KEYS:
subcontroller = controller.get_sub_controllers()[subsystem.title()]
assert isinstance(subcontroller, EigerSubsystemController)
parameters = await subcontroller._introspect_detector_subsystem()
if subsystem == "detector":
# ignored keys should not get added to the controller
Expand All @@ -107,9 +110,9 @@ async def test_controller_groups_and_parameters(sim_eiger_controller: EigerContr
if attr_name == "threshold_energy":
continue
assert attr.group and "Threshold" in attr.group

attr = subcontroller.threshold_1_energy
attr: AttrRW = subcontroller.attributes["threshold_1_energy"] # type: ignore
sender = attr.sender
assert sender is not None
await sender.put(subcontroller, attr, 100.0)
# set parameters to update based on response to put request
assert subcontroller._parameter_updates == {
Expand All @@ -123,8 +126,9 @@ async def test_controller_groups_and_parameters(sim_eiger_controller: EigerContr
subcontroller._parameter_updates.clear()

# make sure API inconsistency for threshold/difference/mode is addressed
attr = subcontroller.threshold_difference_mode
attr: AttrRW = subcontroller.attributes["threshold_difference_mode"] # type: ignore
sender = attr.sender
assert sender is not None
await sender.put(subcontroller, attr, "enabled")
assert subcontroller._parameter_updates == {"threshold/difference/mode"}

Expand Down
8 changes: 6 additions & 2 deletions tests/test_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@
import pytest
from pytest_mock import MockerFixture

from fastcs_eiger.eiger_controller import EigerController, EigerHandler
from fastcs_eiger.eiger_controller import (
EigerController,
EigerDetectorController,
EigerHandler,
)

_lock = asyncio.Lock()

Expand Down Expand Up @@ -71,7 +75,7 @@ async def test_stale_parameter_propagates_to_top_controller(mocker: MockerFixtur
await eiger_controller.initialise()

detector_controller = eiger_controller.get_sub_controllers()["Detector"]

assert isinstance(detector_controller, EigerDetectorController)
# queueing update sets subcontroller to stale
assert detector_controller.stale_parameters.get() is False
await detector_controller.queue_update(["dummy_attribute"])
Expand Down

0 comments on commit 3f39706

Please sign in to comment.