From 057f098147feac24fe7cfd70badb7f38c252ea9b Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Thu, 17 Oct 2024 13:36:14 +0100 Subject: [PATCH 1/6] Add deprecation warning to inject --- src/dodal/common/coordination.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/dodal/common/coordination.py b/src/dodal/common/coordination.py index e04edefc21..5cf61a1c10 100644 --- a/src/dodal/common/coordination.py +++ b/src/dodal/common/coordination.py @@ -1,4 +1,6 @@ import uuid +import warnings +from textwrap import dedent from typing import Any from dodal.common.types import Group @@ -37,4 +39,20 @@ def scan(x: Movable = inject("stage_x"), start: float = 0.0 ...) """ + warnings.warn( + dedent(""" + Inject is deprecated, users are now expected to call the device factory + functions in dodal directly, these will cache devices as singletons after + they have been called once. For example: + + import i22 + + + def my_plan(detector: Readable = i22.saxs()) -> MsgGenerator: + ... + + Where previously the default would have been inject("saxs") + """), + DeprecationWarning, + stacklevel=2, + ) return name From 5202cc5c5dde3a26424ed13f4a1e0d1dad8bc29a Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Fri, 18 Oct 2024 11:48:53 +0100 Subject: [PATCH 2/6] Handle inject deprecation in tests --- src/dodal/common/coordination.py | 2 +- tests/common/test_coordination.py | 28 ++++++++++++++++++++-------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/dodal/common/coordination.py b/src/dodal/common/coordination.py index 5cf61a1c10..de0ed267f5 100644 --- a/src/dodal/common/coordination.py +++ b/src/dodal/common/coordination.py @@ -41,7 +41,7 @@ def scan(x: Movable = inject("stage_x"), start: float = 0.0 ...) warnings.warn( dedent(""" - Inject is deprecated, users are now expected to call the device factory + Inject is deprecated, users are now expected to call the device factory functions in dodal directly, these will cache devices as singletons after they have been called once. For example: diff --git a/tests/common/test_coordination.py b/tests/common/test_coordination.py index 8c0ec6adad..9273bf1551 100644 --- a/tests/common/test_coordination.py +++ b/tests/common/test_coordination.py @@ -17,11 +17,23 @@ def test_group_uid(group: str): def test_type_checking_ignores_inject(): - def example_function(x: Movable = inject("foo")) -> MsgGenerator: # noqa: B008 - yield from {} - - # These asserts are sanity checks - # the real test is whether this test passes type checking - x: Parameter = signature(example_function).parameters["x"] - assert x.annotation == Movable - assert x.default == "foo" + with pytest.raises(DeprecationWarning): + + def example_function(x: Movable = inject("foo")) -> MsgGenerator: # noqa: B008 + yield from {} + + # These asserts are sanity checks + # the real test is whether this test passes type checking + x: Parameter = signature(example_function).parameters["x"] + assert x.annotation == Movable + assert x.default == "foo" + + +def test_inject_is_deprecated(): + with pytest.raises( + DeprecationWarning, + match="Inject is deprecated, users are now expected to call the device factory", + ): + + def example_function(x: Movable = inject("foo")) -> MsgGenerator: # noqa: B008 + yield from {} From 3c062fdc1e8d34345fe7b186116bcff650259d32 Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Fri, 18 Oct 2024 12:06:47 +0100 Subject: [PATCH 3/6] Fix example in deprecation warning --- src/dodal/common/coordination.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/dodal/common/coordination.py b/src/dodal/common/coordination.py index de0ed267f5..9aa8242849 100644 --- a/src/dodal/common/coordination.py +++ b/src/dodal/common/coordination.py @@ -45,9 +45,11 @@ def scan(x: Movable = inject("stage_x"), start: float = 0.0 ...) functions in dodal directly, these will cache devices as singletons after they have been called once. For example: - import i22 + from bluesky.protocols import Readable + from bluesky.utils import MsgGenerator + from dodal.beamlines import i22 - + def my_plan(detector: Readable = i22.saxs()) -> MsgGenerator: + def my_plan(detector: Readable = i22.saxs(connect_immediately=False)) -> MsgGenerator: ... Where previously the default would have been inject("saxs") From d0be5e3b6b9f4e427acc16a10d749e3bb2ed0859 Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Fri, 18 Oct 2024 12:42:10 +0100 Subject: [PATCH 4/6] Draft page about including devices in plans --- docs/how-to/include-devices-in-plans.md | 60 +++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 docs/how-to/include-devices-in-plans.md diff --git a/docs/how-to/include-devices-in-plans.md b/docs/how-to/include-devices-in-plans.md new file mode 100644 index 0000000000..a31d04c830 --- /dev/null +++ b/docs/how-to/include-devices-in-plans.md @@ -0,0 +1,60 @@ +# Include Devices in Plans + +There are three main ways to include dodal devices in plans + +## 1. Pass as Argument + +```python +import bluesky.plans as bp + +from bluesky.protocols import Readable +from bluesky.utils import MsgGenerator +from dodal.beamlines import i22 + +def my_plan(detector: Readable) -> MsgGenerator: + yield from bp.count([detector]) + +RE(my_plan(i22.saxs())) +``` + +This is useful for generic plans that can run on a variety of devices and are not designed with any specific device in mind. + +## 2. Pass as Default Argument + +```python +import bluesky.plans as bp + +from bluesky.protocols import Readable +from bluesky.utils import MsgGenerator +from dodal.beamlines import i22 + +def my_plan(detector: Readable = i22.saxs(connect_immediately=False)) -> MsgGenerator: + yield from bp.count([detector]) + +RE(my_plan())) +``` + +This is useful for plans that will usually, but not exclusively, use the same device. + +## 3. Instantiate Within the Plan + +```python +import bluesky.plans as bp +import ophyd_async.plan_stubs as ops + +from bluesky.protocols import Readable +from bluesky.utils import MsgGenerator +from dodal.beamlines import i22 + +def my_plan() -> MsgGenerator: + detector = i22.saxs(connect_immediately=False) + # We need to connect via the stub instead of directly + # so that the RunEngine performs the connection in the + # correct event loop + yield from ops.ensure_connected(detector) + yield from bp.count([detector]) + +RE(my_plan())) +``` + +This is useful for plans that are designed to only ever work with a specific device. From 72efab235740421c12fa909faf63dde10cc3b1b6 Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Fri, 18 Oct 2024 14:38:21 +0100 Subject: [PATCH 5/6] Remove docs that encourage instantiating devices in the middle of plans --- docs/how-to/include-devices-in-plans.md | 27 ++----------------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/docs/how-to/include-devices-in-plans.md b/docs/how-to/include-devices-in-plans.md index a31d04c830..644265d06d 100644 --- a/docs/how-to/include-devices-in-plans.md +++ b/docs/how-to/include-devices-in-plans.md @@ -1,6 +1,6 @@ # Include Devices in Plans -There are three main ways to include dodal devices in plans +There are two main ways to include dodal devices in plans ## 1. Pass as Argument @@ -34,27 +34,4 @@ def my_plan(detector: Readable = i22.saxs(connect_immediately=False)) -> MsgGene RE(my_plan())) ``` -This is useful for plans that will usually, but not exclusively, use the same device. - -## 3. Instantiate Within the Plan - -```python -import bluesky.plans as bp -import ophyd_async.plan_stubs as ops - -from bluesky.protocols import Readable -from bluesky.utils import MsgGenerator -from dodal.beamlines import i22 - -def my_plan() -> MsgGenerator: - detector = i22.saxs(connect_immediately=False) - # We need to connect via the stub instead of directly - # so that the RunEngine performs the connection in the - # correct event loop - yield from ops.ensure_connected(detector) - yield from bp.count([detector]) - -RE(my_plan())) -``` - -This is useful for plans that are designed to only ever work with a specific device. +This is useful for plans that will usually, but not exclusively, use the same device or that are designed to only ever work with a specific device. From cd50d73472b7f66384b8e2e687e08cc760120c23 Mon Sep 17 00:00:00 2001 From: Callum Forrester Date: Fri, 18 Oct 2024 14:41:44 +0100 Subject: [PATCH 6/6] Fix coverage --- tests/common/test_coordination.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/common/test_coordination.py b/tests/common/test_coordination.py index 9273bf1551..d206d89c80 100644 --- a/tests/common/test_coordination.py +++ b/tests/common/test_coordination.py @@ -16,17 +16,21 @@ def test_group_uid(group: str): assert not gid.endswith(f"{group}-") -def test_type_checking_ignores_inject(): - with pytest.raises(DeprecationWarning): +@pytest.mark.filterwarnings("ignore::DeprecationWarning") +def test_inject_returns_value(): + assert inject("foo") == "foo" - def example_function(x: Movable = inject("foo")) -> MsgGenerator: # noqa: B008 - yield from {} - # These asserts are sanity checks - # the real test is whether this test passes type checking - x: Parameter = signature(example_function).parameters["x"] - assert x.annotation == Movable - assert x.default == "foo" +@pytest.mark.filterwarnings("ignore::DeprecationWarning") +def test_type_checking_ignores_inject(): + def example_function(x: Movable = inject("foo")) -> MsgGenerator: # noqa: B008 + yield from {} + + # These asserts are sanity checks + # the real test is whether this test passes type checking + x: Parameter = signature(example_function).parameters["x"] + assert x.annotation == Movable + assert x.default == "foo" def test_inject_is_deprecated():