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

Resolve some nits in the codebase #708

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
18 changes: 11 additions & 7 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,19 @@ commands =
src = ["src", "tests"]
line-length = 88
lint.select = [
"B", # flake8-bugbear - https://docs.astral.sh/ruff/rules/#flake8-bugbear-b
"C4", # flake8-comprehensions - https://docs.astral.sh/ruff/rules/#flake8-comprehensions-c4
"E", # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e
"F", # pyflakes rules - https://docs.astral.sh/ruff/rules/#pyflakes-f
"W", # pycodestyle warnings - https://docs.astral.sh/ruff/rules/#warning-w
"I", # isort - https://docs.astral.sh/ruff/rules/#isort-i
"UP", # pyupgrade - https://docs.astral.sh/ruff/rules/#pyupgrade-up
"B", # flake8-bugbear - https://docs.astral.sh/ruff/rules/#flake8-bugbear-b
"C4", # flake8-comprehensions - https://docs.astral.sh/ruff/rules/#flake8-comprehensions-c4
"TID252", # flake8-tidy-imports - https://docs.astral.sh/ruff/rules/relative-imports/
"E", # pycodestyle errors - https://docs.astral.sh/ruff/rules/#error-e
"F", # pyflakes rules - https://docs.astral.sh/ruff/rules/#pyflakes-f
"W", # pycodestyle warnings - https://docs.astral.sh/ruff/rules/#warning-w
"I", # isort - https://docs.astral.sh/ruff/rules/#isort-i
"UP", # pyupgrade - https://docs.astral.sh/ruff/rules/#pyupgrade-up
]

[tool.ruff.lint.flake8-tidy-imports]
ban-relative-imports = "all"

[tool.ruff.lint.flake8-bugbear]
extend-immutable-calls = [
"fastapi.Depends",
Expand Down
2 changes: 1 addition & 1 deletion src/blueapi/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
from ._version import __version__
from blueapi._version import __version__

__all__ = ["__version__"]
2 changes: 1 addition & 1 deletion src/blueapi/__main__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .cli.cli import main
from blueapi.cli.cli import main

Check warning on line 1 in src/blueapi/__main__.py

View check run for this annotation

Codecov / codecov/patch

src/blueapi/__main__.py#L1

Added line #L1 was not covered by tests

# test with: python -m blueapi
if __name__ == "__main__":
Expand Down
2 changes: 1 addition & 1 deletion src/blueapi/cli/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
from .cli import main
from blueapi.cli.cli import main

__all__ = ["main"]
5 changes: 2 additions & 3 deletions src/blueapi/cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,15 @@

from blueapi import __version__
from blueapi.cli.format import OutputFormat
from blueapi.cli.scratch import setup_scratch
from blueapi.cli.updates import CliEventRenderer
Comment on lines +18 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

Could: If we're against relative imports as a policy, is there any way we can make the linting fail if they are present?

from blueapi.client.client import BlueapiClient
from blueapi.client.event_bus import AnyEvent, BlueskyStreamingError, EventBusClient
from blueapi.client.rest import BlueskyRemoteControlError
from blueapi.config import ApplicationConfig, ConfigLoader
from blueapi.core import OTLP_EXPORT_ENABLED, DataEvent
from blueapi.worker import ProgressEvent, Task, WorkerEvent

from .scratch import setup_scratch
from .updates import CliEventRenderer


@click.group(invoke_without_command=True)
@click.version_option(version=__version__, prog_name="blueapi")
Expand Down
10 changes: 7 additions & 3 deletions src/blueapi/client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@
start_as_current_span,
)

from blueapi.client.event_bus import (
AnyEvent,
BlueskyStreamingError,
EventBusClient,
OnAnyEvent,
)
from blueapi.client.rest import BlueapiRestClient, BlueskyRemoteControlError
from blueapi.config import ApplicationConfig
from blueapi.core.bluesky_types import DataEvent
from blueapi.service.model import (
Expand All @@ -23,9 +30,6 @@
from blueapi.worker import Task, TrackableTask, WorkerEvent, WorkerState
from blueapi.worker.event import ProgressEvent, TaskStatus

from .event_bus import AnyEvent, BlueskyStreamingError, EventBusClient, OnAnyEvent
from .rest import BlueapiRestClient, BlueskyRemoteControlError

TRACER = get_tracer("client")


Expand Down
8 changes: 4 additions & 4 deletions src/blueapi/core/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from os import environ

from .bluesky_event_loop import configure_bluesky_event_loop
from .bluesky_types import (
from blueapi.core.bluesky_event_loop import configure_bluesky_event_loop
from blueapi.core.bluesky_types import (
BLUESKY_PROTOCOLS,
DataEvent,
Device,
Expand All @@ -13,8 +13,8 @@
is_bluesky_compatible_device_type,
is_bluesky_plan_generator,
)
from .context import BlueskyContext
from .event import EventPublisher, EventStream
from blueapi.core.context import BlueskyContext
from blueapi.core.event import EventPublisher, EventStream

OTLP_EXPORT_ENABLED = environ.get("OTLP_EXPORT_ENABLED") == "true"

Expand Down
2 changes: 1 addition & 1 deletion src/blueapi/core/bluesky_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
)

#: Protocols defining interface to hardware
BLUESKY_PROTOCOLS = list(Device.__args__) # type: ignore
BLUESKY_PROTOCOLS = list(Device.__args__)


def is_bluesky_compatible_device(obj: Any) -> bool:
Expand Down
9 changes: 4 additions & 5 deletions src/blueapi/core/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@
from pydantic_core import CoreSchema, core_schema

from blueapi.config import EnvironmentConfig, SourceKind
from blueapi.utils import BlueapiPlanModelConfig, load_module_all

from .bluesky_types import (
from blueapi.core.bluesky_types import (
BLUESKY_PROTOCOLS,
Device,
HasName,
Expand All @@ -34,7 +32,8 @@
is_bluesky_compatible_device,
is_bluesky_plan_generator,
)
from .device_lookup import find_component
from blueapi.core.device_lookup import find_component
from blueapi.utils import BlueapiPlanModelConfig, load_module_all

LOGGER = logging.getLogger(__name__)

Expand All @@ -61,7 +60,7 @@ def find_device(self, addr: str | list[str]) -> Device | None:
Find a device in this context, allows for recursive search.

Args:
addr (Union[str, List[str]]): Address of the device, examples:
addr (str | list[str]): Address of the device, examples:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I don't tend to bother with type annotations at all in docstrings, since I already put them in the function signatures and AFAIK that's enough for tools like autodoc.

"motors", "motors.x"

Returns:
Expand Down
2 changes: 1 addition & 1 deletion src/blueapi/core/device_lookup.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from typing import Any, TypeVar

from .bluesky_types import Device, is_bluesky_compatible_device
from blueapi.core.bluesky_types import Device, is_bluesky_compatible_device

#: Device obeying Bluesky protocols
D = TypeVar("D", bound=Device)
Expand Down
2 changes: 1 addition & 1 deletion src/blueapi/service/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
from .model import DeviceModel, PlanModel
from blueapi.service.model import DeviceModel, PlanModel

__all__ = ["PlanModel", "DeviceModel"]
9 changes: 4 additions & 5 deletions src/blueapi/service/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@

from blueapi.config import ApplicationConfig
from blueapi.service import interface
from blueapi.worker import Task, TrackableTask, WorkerState
from blueapi.worker.event import TaskStatusEnum

from .model import (
from blueapi.service.model import (
DeviceModel,
DeviceResponse,
EnvironmentResponse,
Expand All @@ -41,7 +38,9 @@
TasksListResponse,
WorkerTask,
)
from .runner import WorkerDispatcher
from blueapi.service.runner import WorkerDispatcher
from blueapi.worker import Task, TrackableTask, WorkerState
from blueapi.worker.event import TaskStatusEnum

REST_API_VERSION = "0.0.5"

Expand Down
2 changes: 1 addition & 1 deletion src/blueapi/startup/example_devices.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from ophyd.sim import Syn2DGauss, SynGauss, SynSignal

from .simmotor import BrokenSynAxis, SynAxisWithMotionEvents
from blueapi.startup.simmotor import BrokenSynAxis, SynAxisWithMotionEvents


def x(name="x") -> SynAxisWithMotionEvents:
Expand Down
15 changes: 9 additions & 6 deletions src/blueapi/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
from .base_model import BlueapiBaseModel, BlueapiModelConfig, BlueapiPlanModelConfig
from .invalid_config_error import InvalidConfigError
from .modules import load_module_all
from .serialization import serialize
from .thread_exception import handle_all_exceptions
from blueapi.utils.base_model import (
BlueapiBaseModel,
BlueapiModelConfig,
BlueapiPlanModelConfig,
)
from blueapi.utils.invalid_config_error import InvalidConfigError
from blueapi.utils.modules import load_module_all
from blueapi.utils.serialization import serialize
from blueapi.utils.thread_exception import handle_all_exceptions

__all__ = [
"handle_all_exceptions",
"load_module_all",
"ConfigLoader",
"serialize",
"BlueapiBaseModel",
"BlueapiModelConfig",
Expand Down
15 changes: 10 additions & 5 deletions src/blueapi/worker/__init__.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
from .event import ProgressEvent, StatusView, TaskStatus, WorkerEvent, WorkerState
from .task import Task
from .task_worker import TaskWorker, TrackableTask
from .worker_errors import WorkerAlreadyStartedError, WorkerBusyError
from blueapi.worker.event import (
ProgressEvent,
StatusView,
TaskStatus,
WorkerEvent,
WorkerState,
)
from blueapi.worker.task import Task
from blueapi.worker.task_worker import TaskWorker, TrackableTask
from blueapi.worker.worker_errors import WorkerAlreadyStartedError, WorkerBusyError

__all__ = [
"TaskWorker",
"Task",
"Worker",
"WorkerEvent",
"WorkerState",
"StatusView",
Expand Down
11 changes: 5 additions & 6 deletions src/blueapi/worker/task_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
from blueapi.core.bluesky_event_loop import configure_bluesky_event_loop
from blueapi.utils.base_model import BlueapiBaseModel
from blueapi.utils.thread_exception import handle_all_exceptions

from .event import (
from blueapi.worker.event import (
ProgressEvent,
RawRunEngineState,
StatusView,
Expand All @@ -42,8 +41,8 @@
WorkerEvent,
WorkerState,
)
from .task import Task
from .worker_errors import WorkerAlreadyStartedError, WorkerBusyError
from blueapi.worker.task import Task
from blueapi.worker.worker_errors import WorkerAlreadyStartedError, WorkerBusyError

LOGGER = logging.getLogger(__name__)
TRACER = get_tracer("task_worker")
Expand Down Expand Up @@ -86,7 +85,7 @@ class TaskWorker:
_state: WorkerState
_errors: list[str]
_warnings: list[str]
_task_channel: Queue # type: ignore
_task_channel: Queue
callumforrester marked this conversation as resolved.
Show resolved Hide resolved
_current: TrackableTask | None
_status_lock: RLock
_status_snapshot: dict[str, StatusView]
Expand Down Expand Up @@ -474,7 +473,7 @@ def on_complete(status: Status) -> None:
del self._status_snapshot[status_uuid]
self._completed_statuses.add(status_uuid)

status.add_callback(on_complete) # type: ignore
status.add_callback(on_complete)

def _on_status_event(
self,
Expand Down
3 changes: 1 addition & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import asyncio
from typing import cast

# Based on https://docs.pytest.org/en/latest/example/simple.html#control-skipping-of-tests-according-to-command-line-option # noqa: E501
import pytest
from bluesky import RunEngine
from bluesky.run_engine import TransitionError
Expand All @@ -12,7 +11,7 @@


@pytest.fixture(scope="function")
def RE(request):
def RE(request: pytest.FixtureRequest) -> RunEngine:
loop = asyncio.new_event_loop()
loop.set_debug(True)
RE = RunEngine({}, call_returns_result=True, loop=loop)
Expand Down
18 changes: 9 additions & 9 deletions tests/system_tests/test_blueapi_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def test_get_plans_by_name(client: BlueapiClient, expected_plans: PlanResponse):
def test_get_non_existent_plan(client: BlueapiClient):
with pytest.raises(KeyError) as exception:
client.get_plan("Not exists")
assert str(exception) == ("{'detail': 'Item not found'}")
assert str(exception) == ("{'detail': 'Item not found'}")


def test_get_devices(client: BlueapiClient, expected_devices: DeviceResponse):
Expand All @@ -80,8 +80,8 @@ def test_get_device_by_name(client: BlueapiClient, expected_devices: DeviceRespo

def test_get_non_existent_device(client: BlueapiClient):
with pytest.raises(KeyError) as exception:
assert client.get_device("Not exists")
assert str(exception) == ("{'detail': 'Item not found'}")
client.get_device("Not exists")
assert str(exception) == ("{'detail': 'Item not found'}")


def test_create_task_and_delete_task_by_id(client: BlueapiClient):
Expand All @@ -92,7 +92,7 @@ def test_create_task_and_delete_task_by_id(client: BlueapiClient):
def test_create_task_validation_error(client: BlueapiClient):
with pytest.raises(KeyError) as exception:
client.create_task(Task(name="Not-exists", params={"Not-exists": 0.0}))
assert str(exception) == ("{'detail': 'Item not found'}")
assert str(exception) == ("{'detail': 'Item not found'}")


def test_get_all_tasks(client: BlueapiClient):
Expand Down Expand Up @@ -128,13 +128,13 @@ def test_get_task_by_id(client: BlueapiClient):
def test_get_non_existent_task(client: BlueapiClient):
with pytest.raises(KeyError) as exception:
client.get_task("Not-exists")
assert str(exception) == "{'detail': 'Item not found'}"
assert str(exception) == "{'detail': 'Item not found'}"


def test_delete_non_existent_task(client: BlueapiClient):
with pytest.raises(KeyError) as exception:
client.clear_task("Not-exists")
assert str(exception) == "{'detail': 'Item not found'}"
assert str(exception) == "{'detail': 'Item not found'}"


def test_put_worker_task(client: BlueapiClient):
Expand All @@ -155,7 +155,7 @@ def test_put_worker_task_fails_if_not_idle(client: BlueapiClient):

with pytest.raises(BlueskyRemoteControlError) as exception:
client.start_task(WorkerTask(task_id=small_task.task_id))
assert str(exception) == "<Response [409]>"
assert str(exception) == "<Response [409]>"
client.abort()
client.clear_task(small_task.task_id)
client.clear_task(long_task.task_id)
Expand All @@ -168,11 +168,11 @@ def test_get_worker_state(client: BlueapiClient):
def test_set_state_transition_error(client: BlueapiClient):
with pytest.raises(BlueskyRemoteControlError) as exception:
client.resume()
assert str(exception) == "<Response [400]>"
assert str(exception) == "<Response [400]>"

with pytest.raises(BlueskyRemoteControlError) as exception:
client.pause()
assert str(exception) == "<Response [400]>"
assert str(exception) == "<Response [400]>"


def test_get_task_by_status(client: BlueapiClient):
Expand Down
Loading
Loading