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

Use ophyd-async 0.9a2 #770

Merged
merged 11 commits into from
Feb 5, 2025
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ dependencies = [
"blueapi >= 0.5.0",
"daq-config-server >= 0.1.1",
"ophyd == 1.9.0",
"ophyd-async >= 0.8a5",
"ophyd-async >= 0.9.0a2",
"bluesky >= 1.13",
"dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git@a2b9d8c20ff3826e3f1407d0e634a4765dde7f70",
"dls-dodal @ git+https://github.com/DiamondLightSource/dodal.git",
]


Expand Down
9 changes: 9 additions & 0 deletions src/mx_bluesky/common/device_setup_plans/setup_panda.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from ophyd_async.core import YamlSettingsProvider
from ophyd_async.fastcs.panda import HDFPanda
from ophyd_async.plan_stubs import apply_panda_settings, retrieve_settings


def load_panda_from_yaml(yaml_directory: str, yaml_file_name: str, panda: HDFPanda):
provider = YamlSettingsProvider(yaml_directory)
settings = yield from retrieve_settings(provider, yaml_file_name, panda)
yield from apply_panda_settings(settings)
12 changes: 12 additions & 0 deletions src/mx_bluesky/common/parameters/constants.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
from enum import Enum

from dodal.devices.aperturescatterguard import ApertureValue
Expand All @@ -6,6 +7,8 @@
from dodal.utils import get_beamline_name
from pydantic.dataclasses import dataclass

from mx_bluesky.definitions import ROOT_DIR

BEAMLINE = get_beamline_name("test")
TEST_MODE = BEAMLINE == "test"

Expand Down Expand Up @@ -118,6 +121,15 @@ class PlanGroupCheckpointConstants:
READY_FOR_OAV = "ready_for_oav"


# Eventually replace below with https://github.com/DiamondLightSource/mx-bluesky/issues/798
@dataclass(frozen=True)
class DeviceSettingsConstants:
PANDA_FLYSCAN_SETTINGS_FILENAME = "panda-gridscan"
PANDA_FLYSCAN_SETTINGS_DIR = os.path.abspath(
f"{ROOT_DIR}/hyperion/resources/panda/{PANDA_FLYSCAN_SETTINGS_FILENAME}"
)


@dataclass(frozen=True)
class SimConstants:
BEAMLINE = "BL03S"
Expand Down
4 changes: 4 additions & 0 deletions src/mx_bluesky/definitions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import os

# Change once Python<3.12 is dropped - see https://github.com/DiamondLightSource/mx-bluesky/issues/798
ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
Copy link
Contributor

Choose a reason for hiding this comment

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

For things which we know are resources contained within our python project, I think I prefer to use importlib.resources to locate them rather than to use file paths. As far as I can tell it seems to be the "proper" way.

Python doesn't guarantee that every module has a __file__ attribute since modules aren't always in files. Having said that I suspect that our packages will probably always be available as files.

I think most of my thoughts in this area derive from analogy with Java ClassLoaders which are a similar concept to python module, and it's best practice to bundle your resources in your jar file (usually in a separate hierarchy), and I think that is the overall intent of things like importlib.resources to encourage this, so that python packaging tools also know how to deploy your resources.

Having said all this

  • importlib.resources does seem to be annoyingly immature as an API
  • there's no functional imperative to use this mechanism at the moment

so I leave this here as an opinion you can choose to follow or ignore...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed in person - as of Python 3.12, we can use this API to get a directory traversible, but we can't use it just yet. I will leave the code as-is but add a comment + issue to use resources again once we drop python < 3.12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #798

14 changes: 7 additions & 7 deletions src/mx_bluesky/hyperion/device_setup_plans/setup_panda.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
from datetime import datetime
from enum import Enum
from importlib import resources
from pathlib import Path

import bluesky.plan_stubs as bps
from bluesky.utils import MsgGenerator
from dodal.common.beamlines.beamline_utils import get_path_provider
from dodal.devices.fast_grid_scan import PandAGridScanParams
from dodal.devices.smargon import Smargon
from ophyd_async.core import load_device
from ophyd_async.fastcs.panda import (
HDFPanda,
SeqTable,
SeqTrigger,
)

import mx_bluesky.hyperion.resources.panda as panda_resource
from mx_bluesky.common.device_setup_plans.setup_panda import load_panda_from_yaml
from mx_bluesky.common.utils.log import LOGGER
from mx_bluesky.hyperion.parameters.constants import DeviceSettingsConstants

MM_TO_ENCODER_COUNTS = 200000
GENERAL_TIMEOUT = 60
Expand Down Expand Up @@ -145,10 +144,11 @@ def setup_panda_for_flyscan(

yield from bps.stage(panda, group="panda-config")

with resources.as_file(
resources.files(panda_resource) / "panda-gridscan.yaml"
) as config_yaml_path:
yield from load_device(panda, str(config_yaml_path))
yield from load_panda_from_yaml(
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the motivation from moving from importlib.resources to passing around paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I could see, importlib.resources was very awkward when needing to specify the directory rather than exact path - and we need the directory now for the load plan

DeviceSettingsConstants.PANDA_FLYSCAN_SETTINGS_DIR,
DeviceSettingsConstants.PANDA_FLYSCAN_SETTINGS_FILENAME,
panda,
)

initial_x = yield from bps.rd(smargon.x.user_readback)
initial_y = yield from bps.rd(smargon.y.user_readback)
Expand Down
2 changes: 2 additions & 0 deletions src/mx_bluesky/hyperion/parameters/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from pydantic.dataclasses import dataclass

from mx_bluesky.common.parameters.constants import (
DeviceSettingsConstants,
DocDescriptorNames,
EnvironmentConstants,
ExperimentParamConstants,
Expand Down Expand Up @@ -60,6 +61,7 @@ class HyperionConstants:
GRAYLOG_PORT = 12232
PARAMETER_SCHEMA_DIRECTORY = "src/hyperion/parameters/schemas/"
LOG_FILE_NAME = "hyperion.log"
DEVICE_SETTINGS_CONSTANTS = DeviceSettingsConstants()


CONST = HyperionConstants()
Loading
Loading