Skip to content

Commit

Permalink
Remove type: ignore where possible
Browse files Browse the repository at this point in the history
  • Loading branch information
DiamondJoseph committed Nov 11, 2024
1 parent dcaa11b commit fa1e077
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 61 deletions.
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
6 changes: 3 additions & 3 deletions src/blueapi/core/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,10 @@ def my_plan(a: int, b: str):
if not is_bluesky_plan_generator(plan):
raise TypeError(f"{plan} is not a valid plan generator function")

model = create_model( # type: ignore
plan.__name__,
model = create_model(
name=plan.__name__,
__config__=BlueapiPlanModelConfig,
**self._type_spec_for_function(plan),
**self._type_spec_for_function(plan), # type: ignore
)
self.plans[plan.__name__] = Plan(
name=plan.__name__, model=model, description=plan.__doc__
Expand Down
4 changes: 2 additions & 2 deletions src/blueapi/worker/task_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class TaskWorker:
_state: WorkerState
_errors: list[str]
_warnings: list[str]
_task_channel: Queue # type: ignore
_task_channel: Queue
_current: TrackableTask | None
_status_lock: RLock
_status_snapshot: dict[str, StatusView]
Expand Down Expand Up @@ -473,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
1 change: 0 additions & 1 deletion 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 Down
8 changes: 4 additions & 4 deletions tests/unit_tests/client/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ def callback(on_event: Callable[[AnyEvent, MessageContext], None]):
on_event(test_event, ctx)
on_event(COMPLETE_EVENT, ctx)

mock_events.subscribe_to_all_events = callback # type: ignore
mock_events.subscribe_to_all_events = callback

mock_on_event = Mock()
client_with_events.run_task(Task(name="foo"), on_event=mock_on_event)
Expand Down Expand Up @@ -414,14 +414,14 @@ def test_run_task_ignores_non_matching_events(
mock_events: MagicMock,
test_event: AnyEvent,
):
mock_rest.create_task.return_value = TaskResponse(task_id="foo") # type: ignore
mock_rest.update_worker_task.return_value = TaskResponse(task_id="foo") # type: ignore
mock_rest.create_task.return_value = TaskResponse(task_id="foo")
mock_rest.update_worker_task.return_value = TaskResponse(task_id="foo")

ctx = Mock()
ctx.correlation_id = "foo"

def callback(on_event: Callable[[AnyEvent, MessageContext], None]):
on_event(test_event, ctx) # type: ignore
on_event(test_event, ctx)
on_event(COMPLETE_EVENT, ctx)

mock_events.subscribe_to_all_events = callback
Expand Down
18 changes: 11 additions & 7 deletions tests/unit_tests/client/test_event_bus.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
from collections.abc import Callable
from typing import Any
from unittest.mock import ANY, Mock

import pytest
from bluesky_stomp.messaging import StompClient

from blueapi.client.event_bus import BlueskyStreamingError, EventBusClient
from blueapi.core.bluesky_types import DataEvent
from blueapi.worker.event import ProgressEvent, WorkerEvent


@pytest.fixture
Expand All @@ -18,7 +22,7 @@ def events(mock_stomp_client: StompClient) -> EventBusClient:

def test_context_manager_connects_and_disconnects(
events: EventBusClient,
mock_stomp_client: Mock,
mock_stomp_client: StompClient,
):
mock_stomp_client.connect.assert_not_called()
mock_stomp_client.disconnect.assert_not_called()
Expand All @@ -32,23 +36,23 @@ def test_context_manager_connects_and_disconnects(

def test_client_subscribes_to_all_events(
events: EventBusClient,
mock_stomp_client: Mock,
mock_stomp_client: StompClient,
):
on_event = Mock
on_event = Mock(spec=Callable[[WorkerEvent | ProgressEvent | DataEvent, Any], None])
with events:
events.subscribe_to_all_events(on_event=on_event) # type: ignore
events.subscribe_to_all_events(on_event=on_event)
mock_stomp_client.subscribe.assert_called_once_with(ANY, on_event)


def test_client_raises_streaming_error_on_subscribe_failure(
events: EventBusClient,
mock_stomp_client: Mock,
mock_stomp_client: StompClient,
):
mock_stomp_client.subscribe.side_effect = RuntimeError("Foo")
on_event = Mock
on_event = Mock(spec=Callable[[WorkerEvent | ProgressEvent | DataEvent, Any], None])
with events:
with pytest.raises(
BlueskyStreamingError,
match="Unable to subscribe to messages from blueapi",
):
events.subscribe_to_all_events(on_event=on_event) # type: ignore
events.subscribe_to_all_events(on_event=on_event)
1 change: 0 additions & 1 deletion tests/unit_tests/core/fake_plan_module.py
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@
from dls_bluesky_core.plans import scan # noqa: F401
59 changes: 27 additions & 32 deletions tests/unit_tests/core/test_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,32 +24,32 @@
#


def has_no_params() -> MsgGenerator: # type: ignore
...
def has_no_params() -> MsgGenerator:
yield from {}


def has_one_param(foo: int) -> MsgGenerator: # type: ignore
...
def has_one_param(foo: int) -> MsgGenerator:
yield from {}


def has_some_params(foo: int = 42, bar: str = "bar") -> MsgGenerator: # type: ignore
...
def has_some_params(foo: int = 42, bar: str = "bar") -> MsgGenerator:
yield from {}


def has_typeless_param(foo) -> MsgGenerator: # type: ignore
...
def has_typeless_param(foo) -> MsgGenerator:
yield from {}


def has_typed_and_typeless_params(foo: int, bar) -> MsgGenerator: # type: ignore
...
def has_typed_and_typeless_params(foo: int, bar) -> MsgGenerator:
yield from {}


def has_typeless_params(foo, bar) -> MsgGenerator: # type: ignore
...
def has_typeless_params(foo, bar) -> MsgGenerator:
yield from {}


def has_default_reference(m: Movable = inject(SIM_MOTOR_NAME)) -> MsgGenerator:
yield from []
yield from {}


MOVABLE_DEFAULT = [inject(SIM_MOTOR_NAME)]
Expand All @@ -58,7 +58,7 @@ def has_default_reference(m: Movable = inject(SIM_MOTOR_NAME)) -> MsgGenerator:
def has_default_nested_reference(
m: list[Movable] = MOVABLE_DEFAULT,
) -> MsgGenerator:
yield from []
yield from {}


#
Expand Down Expand Up @@ -102,12 +102,11 @@ def devicey_context(sim_motor: SynAxis, sim_detector: SynGauss) -> BlueskyContex


class SomeConfigurable:
def read_configuration(self) -> SyncOrAsync[dict[str, Reading]]: # type: ignore
...
def read_configuration(self) -> SyncOrAsync[dict[str, Reading]]:
return {}

def describe_configuration( # type: ignore
self,
) -> SyncOrAsync[dict[str, Descriptor]]: ...
def describe_configuration(self) -> SyncOrAsync[dict[str, Descriptor]]:
return {}


@pytest.fixture
Expand All @@ -124,11 +123,11 @@ def test_add_plan(empty_context: BlueskyContext, plan: PlanGenerator) -> None:
def test_generated_schema(
empty_context: BlueskyContext,
):
def demo_plan(foo: int, mov: Movable) -> MsgGenerator: # type: ignore
...
def demo_plan(foo: int, mov: Movable) -> MsgGenerator:
yield from {}

empty_context.register_plan(demo_plan)
schema = empty_context.plans["demo_plan"].model.schema()
schema = empty_context.plans["demo_plan"].model.model_json_schema()
assert schema["properties"] == {
"foo": {"title": "Foo", "type": "integer"},
"mov": {"title": "Mov", "type": "bluesky.protocols.Movable"},
Expand Down Expand Up @@ -246,12 +245,12 @@ def test_lookup_non_device(devicey_context: BlueskyContext) -> None:

def test_add_non_plan(empty_context: BlueskyContext) -> None:
with pytest.raises(TypeError):
empty_context.register_plan("not a plan") # type: ignore
empty_context.register_plan("not a plan")


def test_add_non_device(empty_context: BlueskyContext) -> None:
with pytest.raises(TypeError):
empty_context.register_device("not a device") # type: ignore
empty_context.register_device("not a device")


def test_add_devices_and_plans_from_modules_with_config(
Expand Down Expand Up @@ -310,24 +309,20 @@ def test_reference_type_conversion(empty_context: BlueskyContext) -> None:
def test_reference_type_conversion_union(empty_context: BlueskyContext) -> None:
movable_ref: type = empty_context._reference(Movable)
assert empty_context._convert_type(Movable) == movable_ref
assert (
empty_context._convert_type(Union[Movable, int]) == Union[movable_ref, int] # noqa # type: ignore
)
assert empty_context._convert_type(Union[Movable, int]) == Union[movable_ref, int]


def test_reference_type_conversion_new_style_union(
empty_context: BlueskyContext,
) -> None:
movable_ref: type = empty_context._reference(Movable)
assert empty_context._convert_type(Movable) == movable_ref
assert (
empty_context._convert_type(Movable | int) == movable_ref | int # type: ignore
)
assert empty_context._convert_type(Movable | int) == movable_ref | int


def test_default_device_reference(empty_context: BlueskyContext) -> None:
def default_movable(mov: Movable = "demo") -> MsgGenerator: # type: ignore
...
def default_movable(mov: Movable = inject("demo")) -> MsgGenerator:
yield from {}

spec = empty_context._type_spec_for_function(default_movable)
movable_ref = empty_context._reference(Movable)
Expand Down
23 changes: 15 additions & 8 deletions tests/unit_tests/service/test_rest_api.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import uuid
from collections.abc import Iterator
from dataclasses import dataclass
from typing import Any
from unittest.mock import MagicMock, patch

import pytest
Expand Down Expand Up @@ -171,14 +172,20 @@ class MyModel(BaseModel):

plan = Plan(name="my-plan", model=MyModel)
get_plan_mock.return_value = PlanModel.from_plan(plan)
submit_task_mock.side_effect = ValidationError.from_exception_data(
title="ValueError",
line_errors=[
InitErrorDetails(
type="missing", loc=("id",), msg="value is required for Identifier"
) # type: ignore
],
)

def raise_validation_error(bar: Any):
return ValidationError.from_exception_data(
title="ValueError",
line_errors=[
InitErrorDetails(
input=bar,
type="missing",
loc=("id",),
)
],
)

submit_task_mock.side_effect = raise_validation_error
response = client.post("/tasks", json={"name": "my-plan"})
assert response.status_code == 422
assert response.json() == {
Expand Down
4 changes: 2 additions & 2 deletions tests/unit_tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,11 +322,11 @@ def test_config_yaml_parsed_complete(temp_yaml_config_file: dict):
assert loaded_config.stomp.auth is not None
assert (
loaded_config.stomp.auth.password.get_secret_value()
== config_data["stomp"]["auth"]["password"] # noqa: E501
== config_data["stomp"]["auth"]["password"]
)
# Remove the password field to not compare it again in the full dict comparison
del target_dict_json["stomp"]["auth"]["password"]
del config_data["stomp"]["auth"]["password"] # noqa: E501
del config_data["stomp"]["auth"]["password"]
# Assert that the remaining config data is identical
assert (
target_dict_json == config_data
Expand Down

0 comments on commit fa1e077

Please sign in to comment.