diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 21d449cf..6a174a04 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -74,10 +74,7 @@ jobs: path: ./coverage.lcov - name: Analyse with MyPy - run: mypy src - - - name: Type tests with MyPy - run: mypy --warn-unused-ignores typing_tests + run: mypy test-with-unpinned-deps: runs-on: ubuntu-latest @@ -105,7 +102,7 @@ jobs: - name: Analyse with MyPy if: success() || failure() - run: mypy src + run: mypy - name: Test with pytest if: success() || failure() diff --git a/pyproject.toml b/pyproject.toml index d1f03472..7b002e3f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -108,7 +108,12 @@ property-decorators = [ ] [tool.mypy] +files = ["src/", "typing_tests/"] plugins = ["pydantic.mypy"] +disallow_untyped_defs = true +check_untyped_defs = true +disallow_untyped_decorators = true +warn_unused_ignores = true [tool.flake8] diff --git a/src/labthings_fastapi/actions/__init__.py b/src/labthings_fastapi/actions/__init__.py index 81e5c738..b526ea23 100644 --- a/src/labthings_fastapi/actions/__init__.py +++ b/src/labthings_fastapi/actions/__init__.py @@ -168,7 +168,7 @@ def status(self) -> InvocationStatus: return self._status @property - def action(self): + def action(self) -> ActionDescriptor: """The `.ActionDescriptor` object running in this thread.""" action = self.action_ref() assert action is not None, "The action for an `Invocation` has been deleted!" @@ -210,10 +210,12 @@ def response(self, request: Optional[Request] = None) -> InvocationModel: LinkElement(rel="self", href=href), LinkElement(rel="output", href=href + "/output"), ] - return self.action.invocation_model( + # The line below confuses MyPy because self.action **evaluates to** a Descriptor + # object (i.e. we don't call __get__ on the descriptor). + return self.action.invocation_model( # type: ignore[call-overload] status=self.status, id=self.id, - action=self.thing.path + self.action.name, + action=self.thing.path + self.action.name, # type: ignore[call-overload] href=href, timeStarted=self._start_time, timeCompleted=self._end_time, @@ -249,24 +251,25 @@ def run(self) -> None: See `.Invocation.status` for status values. """ + # self.action evaluates to an ActionDescriptor. This confuses mypy, + # which thinks we are calling ActionDescriptor.__get__. + action: ActionDescriptor = self.action # type: ignore[call-overload] try: - self.action.emit_changed_event(self.thing, self._status) + action.emit_changed_event(self.thing, self._status.value) # Capture just this thread's log messages handler = DequeLogHandler(dest=self._log) logger = invocation_logger(self.id) logger.addHandler(handler) - action = self.action thing = self.thing kwargs = model_to_dict(self.input) - assert action is not None assert thing is not None with self._status_lock: self._status = InvocationStatus.RUNNING self._start_time = datetime.datetime.now() - self.action.emit_changed_event(self.thing, self._status) + action.emit_changed_event(self.thing, self._status.value) # The next line actually runs the action. ret = action.__get__(thing)(**kwargs, **self.dependencies) @@ -274,12 +277,12 @@ def run(self) -> None: with self._status_lock: self._return_value = ret self._status = InvocationStatus.COMPLETED - self.action.emit_changed_event(self.thing, self._status) + action.emit_changed_event(self.thing, self._status.value) except InvocationCancelledError: logger.info(f"Invocation {self.id} was cancelled.") with self._status_lock: self._status = InvocationStatus.CANCELLED - self.action.emit_changed_event(self.thing, self._status) + action.emit_changed_event(self.thing, self._status.value) except Exception as e: # skipcq: PYL-W0703 # First log if isinstance(e, InvocationError): @@ -291,7 +294,7 @@ def run(self) -> None: with self._status_lock: self._status = InvocationStatus.ERROR self._exception = e - self.action.emit_changed_event(self.thing, self._status) + action.emit_changed_event(self.thing, self._status.value) finally: with self._status_lock: self._end_time = datetime.datetime.now() @@ -341,9 +344,9 @@ def emit(self, record: logging.LogRecord) -> None: class ActionManager: """A class to manage a collection of actions.""" - def __init__(self): + def __init__(self) -> None: """Set up an `.ActionManager`.""" - self._invocations = {} + self._invocations: dict[uuid.UUID, Invocation] = {} self._invocations_lock = Lock() @property @@ -443,10 +446,13 @@ def list_invocations( i.response(request=request) for i in self.invocations if thing is None or i.thing == thing - if action is None or i.action == action + if action is None or i.action == action # type: ignore[call-overload] + # i.action evaluates to an ActionDescriptor, which confuses mypy - it + # thinks we are calling ActionDescriptor.__get__ but this isn't ever + # called. ] - def expire_invocations(self): + def expire_invocations(self) -> None: """Delete invocations that have passed their expiry time.""" to_delete = [] with self._invocations_lock: @@ -465,7 +471,9 @@ def attach_to_app(self, app: FastAPI) -> None: """ @app.get(ACTION_INVOCATIONS_PATH, response_model=list[InvocationModel]) - def list_all_invocations(request: Request, _blob_manager: BlobIOContextDep): + def list_all_invocations( + request: Request, _blob_manager: BlobIOContextDep + ) -> list[InvocationModel]: return self.list_invocations(request=request) @app.get( @@ -512,7 +520,9 @@ def action_invocation( 503: {"description": "No result is available for this invocation"}, }, ) - def action_invocation_output(id: uuid.UUID, _blob_manager: BlobIOContextDep): + def action_invocation_output( + id: uuid.UUID, _blob_manager: BlobIOContextDep + ) -> Any: """Get the output of an action invocation. This returns just the "output" component of the action invocation. If the diff --git a/src/labthings_fastapi/client/__init__.py b/src/labthings_fastapi/client/__init__.py index 320c9fd8..9c5ce620 100644 --- a/src/labthings_fastapi/client/__init__.py +++ b/src/labthings_fastapi/client/__init__.py @@ -148,7 +148,7 @@ def get_property(self, path: str) -> Any: r.raise_for_status() return r.json() - def set_property(self, path: str, value: Any): + def set_property(self, path: str, value: Any) -> None: """Make a PUT request to set the value of a property. :param path: the URI of the ``getproperty`` endpoint, relative @@ -159,7 +159,7 @@ def set_property(self, path: str, value: Any): r = self.client.put(urljoin(self.path, path), json=value) r.raise_for_status() - def invoke_action(self, path: str, **kwargs): + def invoke_action(self, path: str, **kwargs: Any) -> Any: r"""Invoke an action on the Thing. This method will make the initial POST request to invoke an action, @@ -180,8 +180,17 @@ def invoke_action(self, path: str, **kwargs): :raise RuntimeError: is raised if the action does not complete successfully. """ for k in kwargs.keys(): - if isinstance(kwargs[k], ClientBlobOutput): - kwargs[k] = {"href": kwargs[k].href, "media_type": kwargs[k].media_type} + value = kwargs[k] + if isinstance(value, ClientBlobOutput): + # ClientBlobOutput objects may be used as input to a subsequent + # action. When this is done, they should be serialised to a dict + # with `href` and `media_type` keys, as done below. + # Ideally this should be replaced with `Blob` and the use of + # `pydantic` models to serialise action inputs. + # + # Note that the blob will not be uploaded: we rely on the blob + # still existing on the server. + kwargs[k] = {"href": value.href, "media_type": value.media_type} r = self.client.post(urljoin(self.path, path), json=kwargs) r.raise_for_status() invocation = poll_invocation(self.client, r.json()) @@ -270,7 +279,9 @@ class Client(cls): # type: ignore[valid-type, misc] class PropertyClientDescriptor: """A base class for properties on `.ThingClient` objects.""" - pass + name: str + type: type | BaseModel + path: str def property_descriptor( @@ -312,10 +323,10 @@ class P(PropertyClientDescriptor): if readable: def __get__( - self, + self: PropertyClientDescriptor, obj: Optional[ThingClient] = None, _objtype: Optional[type[ThingClient]] = None, - ): + ) -> Any: if obj is None: return self return obj.get_property(self.name) @@ -324,7 +335,9 @@ def __get__( P.__get__ = __get__ # type: ignore[attr-defined] if writeable: - def __set__(self, obj: ThingClient, value: Any): + def __set__( + self: PropertyClientDescriptor, obj: ThingClient, value: Any + ) -> None: obj.set_property(self.name, value) __set__.__annotations__["value"] = model @@ -349,7 +362,7 @@ def add_action(cls: type[ThingClient], action_name: str, action: dict) -> None: format. """ - def action_method(self, **kwargs): + def action_method(self: ThingClient, **kwargs: Any) -> Any: return self.invoke_action(action_name, **kwargs) if "output" in action and "type" in action["output"]: @@ -359,7 +372,7 @@ def action_method(self, **kwargs): setattr(cls, action_name, action_method) -def add_property(cls: type[ThingClient], property_name: str, property: dict): +def add_property(cls: type[ThingClient], property_name: str, property: dict) -> None: """Add a property to a ThingClient subclass. A descriptor will be added to the provided class that makes the diff --git a/src/labthings_fastapi/client/in_server.py b/src/labthings_fastapi/client/in_server.py index 7bcdfbe7..9c3d6ff6 100644 --- a/src/labthings_fastapi/client/in_server.py +++ b/src/labthings_fastapi/client/in_server.py @@ -113,18 +113,22 @@ class P(PropertyClientDescriptor): path = property_path or property_name def __get__( - self, + self: PropertyClientDescriptor, obj: Optional[DirectThingClient] = None, _objtype: Optional[type[DirectThingClient]] = None, - ): + ) -> Any: if obj is None: return self return getattr(obj._wrapped_thing, self.name) - def __set__(self, obj: DirectThingClient, value: Any): + def __set__( + self: PropertyClientDescriptor, obj: DirectThingClient, value: Any + ) -> None: setattr(obj._wrapped_thing, self.name, value) - def set_readonly(self, obj: DirectThingClient, value: Any): + def set_readonly( + self: PropertyClientDescriptor, obj: DirectThingClient, value: Any + ) -> None: raise AttributeError("This property is read-only.") if readable: @@ -198,7 +202,7 @@ def add_action( """ @wraps(action.func) - def action_method(self, **kwargs): + def action_method(self: DirectThingClient, **kwargs: Any) -> Any: dependency_kwargs = { param.name: self._dependencies[param.name] for param in action.dependency_params @@ -273,7 +277,7 @@ def direct_thing_client_class( def init_proxy( self: DirectThingClient, request: Request, **dependencies: Mapping[str, Any] - ): + ) -> None: r"""Initialise a DirectThingClient (this docstring will be replaced). :param self: The DirectThingClient instance we're initialising. diff --git a/src/labthings_fastapi/decorators/__init__.py b/src/labthings_fastapi/decorators/__init__.py index b1950b76..f1c61460 100644 --- a/src/labthings_fastapi/decorators/__init__.py +++ b/src/labthings_fastapi/decorators/__init__.py @@ -37,7 +37,7 @@ """ from functools import wraps, partial -from typing import Optional, Callable, overload +from typing import Any, Optional, Callable, overload from ..descriptors import ( ActionDescriptor, EndpointDescriptor, @@ -45,7 +45,7 @@ ) -def mark_thing_action(func: Callable, **kwargs) -> ActionDescriptor: +def mark_thing_action(func: Callable, **kwargs: Any) -> ActionDescriptor: r"""Mark a method of a Thing as an Action. We replace the function with a descriptor that's a @@ -65,12 +65,12 @@ class ActionDescriptorSubclass(ActionDescriptor): @overload -def thing_action(func: Callable, **kwargs) -> ActionDescriptor: ... +def thing_action(func: Callable, **kwargs: Any) -> ActionDescriptor: ... @overload def thing_action( - **kwargs, + **kwargs: Any, ) -> Callable[ [ Callable, @@ -81,7 +81,7 @@ def thing_action( @wraps(mark_thing_action) def thing_action( - func: Optional[Callable] = None, **kwargs + func: Optional[Callable] = None, **kwargs: Any ) -> ( ActionDescriptor | Callable[ @@ -121,7 +121,7 @@ def thing_action( def fastapi_endpoint( - method: HTTPMethod, path: Optional[str] = None, **kwargs + method: HTTPMethod, path: Optional[str] = None, **kwargs: Any ) -> Callable[[Callable], EndpointDescriptor]: r"""Mark a function as a FastAPI endpoint without making it an action. diff --git a/src/labthings_fastapi/dependencies/invocation.py b/src/labthings_fastapi/dependencies/invocation.py index 9777df40..85b0bf6e 100644 --- a/src/labthings_fastapi/dependencies/invocation.py +++ b/src/labthings_fastapi/dependencies/invocation.py @@ -146,7 +146,7 @@ def __init__(self, id: InvocationID): threading.Event.__init__(self) self.invocation_id = id - def raise_if_set(self): + def raise_if_set(self) -> None: """Raise an exception if the event is set. This is intended as a compact alternative to: @@ -161,7 +161,7 @@ def raise_if_set(self): if self.is_set(): raise InvocationCancelledError("The action was cancelled.") - def sleep(self, timeout: float): + def sleep(self, timeout: float) -> None: r"""Sleep for a given time in seconds, but raise an exception if cancelled. This function can be used in place of `time.sleep`. It will usually behave diff --git a/src/labthings_fastapi/descriptors/action.py b/src/labthings_fastapi/descriptors/action.py index 68a409a3..7c06d899 100644 --- a/src/labthings_fastapi/descriptors/action.py +++ b/src/labthings_fastapi/descriptors/action.py @@ -122,11 +122,11 @@ def __init__( self.invocation_model.__name__ = f"{self.name}_invocation" @overload - def __get__(self, obj: Literal[None], type=None) -> ActionDescriptor: # noqa: D105 + def __get__(self, obj: Literal[None], type: type[Thing]) -> ActionDescriptor: # noqa: D105 ... @overload - def __get__(self, obj: Thing, type=None) -> Callable: # noqa: D105 + def __get__(self, obj: Thing, type: type[Thing] | None = None) -> Callable: # noqa: D105 ... def __get__( @@ -158,17 +158,17 @@ def __get__( return partial(self.func, obj) @property - def name(self): + def name(self) -> str: """The name of the wrapped function.""" return self.func.__name__ @property - def title(self): + def title(self) -> str: """A human-readable title.""" return get_summary(self.func) or self.name @property - def description(self): + def description(self) -> str | None: """A description of the action.""" return get_docstring(self.func, remove_summary=True) @@ -189,7 +189,7 @@ def _observers_set(self, obj: Thing) -> WeakSet: ld.action_observers[self.name] = WeakSet() return ld.action_observers[self.name] - def emit_changed_event(self, obj: Thing, status: str): + def emit_changed_event(self, obj: Thing, status: str) -> None: """Notify subscribers that the action status has changed. This function is run from within the `.Invocation` thread that @@ -266,12 +266,12 @@ def start_action( action_manager: ActionManagerContextDep, _blob_manager: BlobIOContextDep, request: Request, - body, + body: Any, # This annotation will be overwritten below. id: InvocationID, cancel_hook: CancelHook, background_tasks: BackgroundTasks, - **dependencies, - ): + **dependencies: Any, + ) -> InvocationModel: action = action_manager.invoke_action( action=self, thing=thing, @@ -318,6 +318,7 @@ def start_action( if hasattr(self.output_model, "media_type"): responses[200]["content"][self.output_model.media_type] = {} # Now we can add the endpoint to the app. + assert thing.path is not None, "Can't add the endpoint without thing.path!" app.post( thing.path + self.name, response_model=self.invocation_model, @@ -341,7 +342,7 @@ def start_action( ) def list_invocations( action_manager: ActionManagerContextDep, _blob_manager: BlobIOContextDep - ): + ) -> list[InvocationModel]: return action_manager.list_invocations(self, thing) def action_affordance( @@ -359,6 +360,7 @@ def action_affordance( :return: An `.ActionAffordance` describing this action. """ path = path or thing.path + assert path is not None, "Can't generate forms without a path!" forms = [ Form[ActionOp](href=path + self.name, op=[ActionOp.invokeaction]), ] diff --git a/src/labthings_fastapi/descriptors/endpoint.py b/src/labthings_fastapi/descriptors/endpoint.py index 6beec6a1..47c542c7 100644 --- a/src/labthings_fastapi/descriptors/endpoint.py +++ b/src/labthings_fastapi/descriptors/endpoint.py @@ -19,6 +19,7 @@ from ..utilities.introspection import get_docstring, get_summary from typing import ( + Any, Callable, Literal, Mapping, @@ -45,7 +46,7 @@ def __init__( func: Callable, http_method: HTTPMethod = "get", path: Optional[str] = None, - **kwargs: Mapping, + **kwargs: Mapping[str, Any], ): r"""Initialise an EndpointDescriptor. @@ -67,10 +68,10 @@ def __init__( self.kwargs = kwargs @overload - def __get__(self, obj: Literal[None], type=None) -> Self: ... # noqa: D105 + def __get__(self, obj: Literal[None], type: type[Thing] | None = None) -> Self: ... # noqa: D105 @overload - def __get__(self, obj: Thing, type=None) -> Callable: ... # noqa: D105 + def __get__(self, obj: Thing, type: type[Thing] | None = None) -> Callable: ... # noqa: D105 def __get__( self, obj: Optional[Thing], type: type[Thing] | None = None @@ -96,26 +97,26 @@ def __get__( return wraps(self.func)(partial(self.func, obj)) @property - def name(self): + def name(self) -> str: """The name of the wrapped function.""" return self.func.__name__ @property - def path(self): + def path(self) -> str: """The path of the endpoint (relative to the Thing).""" return self._path or self.name @property - def title(self): + def title(self) -> str: """A human-readable title.""" return get_summary(self.func) or self.name @property - def description(self): + def description(self) -> str | None: """A description of the endpoint.""" return get_docstring(self.func, remove_summary=True) - def add_to_fastapi(self, app: FastAPI, thing: Thing): + def add_to_fastapi(self, app: FastAPI, thing: Thing) -> None: """Add an endpoint for this function to a FastAPI app. We will add an endpoint to the app, bound to a particular `.Thing`. @@ -126,11 +127,12 @@ def add_to_fastapi(self, app: FastAPI, thing: Thing): :param app: the `fastapi.FastAPI` application we are adding to. :param thing: the `.Thing` we're bound to. """ + assert thing.path is not None # fastapi_endpoint is equivalent to app.get/app.post/whatever fastapi_endpoint = getattr(app, self.http_method) bound_function = partial(self.func, thing) # NB the line above can't use self.__get__ as wraps() confuses FastAPI - kwargs = { # Auto-populate description and summary + kwargs: dict[str, Any] = { # Auto-populate description and summary "description": f"## {self.title}\n\n {self.description}", "summary": self.title, } diff --git a/src/labthings_fastapi/example_things/__init__.py b/src/labthings_fastapi/example_things/__init__.py index def9bb57..9d6daba3 100644 --- a/src/labthings_fastapi/example_things/__init__.py +++ b/src/labthings_fastapi/example_things/__init__.py @@ -74,7 +74,7 @@ def make_a_dict( return out @thing_action - def increment_counter(self): + def increment_counter(self) -> None: """Increment the counter property. This action doesn't do very much - all it does, in fact, @@ -84,7 +84,7 @@ def increment_counter(self): self.counter += 1 @thing_action - def slowly_increase_counter(self, increments: int = 60, delay: float = 1): + def slowly_increase_counter(self, increments: int = 60, delay: float = 1) -> None: """Increment the counter slowly over a minute. :param increments: how many times to increment. @@ -118,7 +118,7 @@ class ThingWithBrokenAffordances(Thing): """A Thing that raises exceptions in actions/properties.""" @thing_action - def broken_action(self): + def broken_action(self) -> None: """Do something that raises an exception. :raise RuntimeError: every time. @@ -126,7 +126,7 @@ def broken_action(self): raise RuntimeError("This is a broken action") @lt_property - def broken_property(self): + def broken_property(self) -> None: """Raise an exception when the property is accessed. :raise RuntimeError: every time. @@ -137,7 +137,7 @@ def broken_property(self): class ThingThatCantInstantiate(Thing): """A Thing that raises an exception in __init__.""" - def __init__(self): + def __init__(self) -> None: """Fail to initialise. :raise RuntimeError: every time. @@ -148,14 +148,14 @@ def __init__(self): class ThingThatCantStart(Thing): """A Thing that raises an exception in __enter__.""" - def __enter__(self): + def __enter__(self) -> None: """Fail to start the thing. :raise RuntimeError: every time. """ raise RuntimeError("This thing can't start") - def __exit__(self, exc_t: Any, exc_v: Any, exc_tb: Any): + def __exit__(self, exc_t: Any, exc_v: Any, exc_tb: Any) -> None: """Don't leave the thing as we never entered. :param exc_t: Exception type. diff --git a/src/labthings_fastapi/outputs/blob.py b/src/labthings_fastapi/outputs/blob.py index 420ad7f8..201dfff5 100644 --- a/src/labthings_fastapi/outputs/blob.py +++ b/src/labthings_fastapi/outputs/blob.py @@ -46,6 +46,7 @@ def get_image(self) -> MyImageBlob: import shutil from typing import ( Annotated, + Any, AsyncGenerator, Callable, Literal, @@ -215,7 +216,7 @@ class BlobFile: id: Optional[uuid.UUID] = None """A unique ID to identify the data in a `.BlobManager`.""" - def __init__(self, file_path: str, media_type: str, **kwargs): + def __init__(self, file_path: str, media_type: str, **kwargs: Any): r"""Create a `.BlobFile` to wrap data stored on disk. `.BlobFile` objects wrap data stored on disk as files. They diff --git a/src/labthings_fastapi/outputs/mjpeg_stream.py b/src/labthings_fastapi/outputs/mjpeg_stream.py index 3b0d3300..efdd882e 100644 --- a/src/labthings_fastapi/outputs/mjpeg_stream.py +++ b/src/labthings_fastapi/outputs/mjpeg_stream.py @@ -139,7 +139,7 @@ def __init__(self, ringbuffer_size: int = 10): self._ringbuffer: list[RingbufferEntry] = [] self.reset(ringbuffer_size=ringbuffer_size) - def reset(self, ringbuffer_size: Optional[int] = None): + def reset(self, ringbuffer_size: Optional[int] = None) -> None: """Reset the stream and optionally change the ringbuffer size. Discard all frames from the ringbuffer and reset the frame index. @@ -361,7 +361,7 @@ class MJPEGStreamDescriptor: This descriptor does not currently show up in the :ref:`wot_td`. """ - def __init__(self, **kwargs: dict[str, Any]): + def __init__(self, **kwargs: Any): r"""Initialise an MJPEGStreamDescriptor. :param \**kwargs: keyword arguments are passed to the initialiser of @@ -381,10 +381,10 @@ def __set_name__(self, _owner: Thing, name: str) -> None: self.name = name @overload - def __get__(self, obj: Literal[None], type=None) -> Self: ... # noqa: D105 + def __get__(self, obj: Literal[None], type: type | None = None) -> Self: ... # noqa: D105 @overload - def __get__(self, obj: Thing, type=None) -> MJPEGStream: ... # noqa: D105 + def __get__(self, obj: Thing, type: type | None = None) -> MJPEGStream: ... # noqa: D105 def __get__( self, obj: Optional[Thing], type: type[Thing] | None = None @@ -452,5 +452,5 @@ class Camera(lt.Thing): f"{thing.path}{self.name}/viewer", response_class=HTMLResponse, ) - async def viewer_page(): + async def viewer_page() -> HTMLResponse: return await self.viewer_page(f"{thing.path}{self.name}") diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py index 9af1bbf9..a88cbe47 100644 --- a/src/labthings_fastapi/properties.py +++ b/src/labthings_fastapi/properties.py @@ -381,7 +381,7 @@ def add_to_fastapi(self, app: FastAPI, thing: Thing) -> None: # the function to the decorator. if not self.readonly: - def set_property(body): # We'll annotate body later + def set_property(body: Any) -> None: # We'll annotate body later if isinstance(body, RootModel): body = body.root return self.__set__(thing, body) @@ -402,7 +402,7 @@ def set_property(body): # We'll annotate body later summary=self.title, description=f"## {self.title}\n\n{self.description or ''}", ) - def get_property(): + def get_property() -> Any: return self.__get__(thing) def property_affordance( @@ -445,6 +445,19 @@ def property_affordance( } ) + def __set__(self, obj: Thing, value: Any) -> None: + """Set the property (stub method). + + This is a stub ``__set__`` method to mark this as a data descriptor. + + :param obj: The Thing on which we are setting the value. + :param value: The new value for the Thing. + :raises NotImplementedError: as this must be overridden by concrete classes. + """ + raise NotImplementedError( + "__set__ must be overridden by property implementations." + ) + class DataProperty(BaseProperty[Value], Generic[Value]): """A Property descriptor that acts like a regular variable. @@ -602,7 +615,7 @@ def __set__( if emit_changed_event: self.emit_changed_event(obj, value) - def _observers_set(self, obj: Thing): + def _observers_set(self, obj: Thing) -> WeakSet: """Return the observers of this property. Each observer in this set will be notified when the property is changed. @@ -646,7 +659,7 @@ def emit_changed_event(self, obj: Thing, value: Value) -> None: value, ) - async def emit_changed_event_async(self, obj: Thing, value: Value): + async def emit_changed_event_async(self, obj: Thing, value: Value) -> None: """Notify subscribers that the property has changed. This function may only be run in the `anyio` event loop. See @@ -793,7 +806,7 @@ def instance_get(self, obj: Thing) -> Value: """ return self.fget(obj) - def __set__(self, obj: Thing, value: Value): + def __set__(self, obj: Thing, value: Value) -> None: """Set the value of the property. :param obj: the `.Thing` on which the attribute is accessed. diff --git a/src/labthings_fastapi/server/__init__.py b/src/labthings_fastapi/server/__init__.py index 0ef9abd9..12763d7e 100644 --- a/src/labthings_fastapi/server/__init__.py +++ b/src/labthings_fastapi/server/__init__.py @@ -216,7 +216,7 @@ async def lifespan(self, app: FastAPI) -> AsyncGenerator[None]: self.blocking_portal = None - def add_things_view_to_app(self): + def add_things_view_to_app(self) -> None: """Add an endpoint that shows the list of attached things.""" thing_server = self diff --git a/src/labthings_fastapi/server/cli.py b/src/labthings_fastapi/server/cli.py index 5a1aefbf..d8e32ba2 100644 --- a/src/labthings_fastapi/server/cli.py +++ b/src/labthings_fastapi/server/cli.py @@ -30,7 +30,7 @@ from . import ThingServer, server_from_config -def get_default_parser(): +def get_default_parser() -> ArgumentParser: """Return the default CLI parser for LabThings. This can be used to add more arguments, for custom CLIs that make use of diff --git a/src/labthings_fastapi/thing.py b/src/labthings_fastapi/thing.py index 3d3cb739..c2ca49d8 100644 --- a/src/labthings_fastapi/thing.py +++ b/src/labthings_fastapi/thing.py @@ -152,7 +152,7 @@ def thing_description(request: Request) -> ThingDescription: return self.thing_description(base=str(request.base_url)) @server.app.websocket(self.path + "ws") - async def websocket(ws: WebSocket): + async def websocket(ws: WebSocket) -> None: await websocket_endpoint(self, ws) # A private variable to hold the list of settings so it doesn't need to be @@ -230,13 +230,14 @@ def load_settings(self, setting_storage_path: str) -> None: _LOGGER.warning("Error loading settings for %s", thing_name) self._setting_storage_path = setting_storage_path - def save_settings(self): + def save_settings(self) -> None: """Save settings to JSON. This is called whenever a setting is updated. All settings are written to the settings file every time. """ if self._settings is not None: + assert self._setting_storage_path is not None setting_dict = {} for name in self._settings.keys(): value = getattr(self, name) @@ -352,7 +353,7 @@ def observe_property(self, property_name: str, stream: ObjectSendStream) -> None raise KeyError(f"{property_name} is not a LabThings Property") prop._observers_set(self).add(stream) - def observe_action(self, action_name: str, stream: ObjectSendStream): + def observe_action(self, action_name: str, stream: ObjectSendStream) -> None: """Register a stream to receive action status change notifications. :param action_name: the action to register for. diff --git a/src/labthings_fastapi/thing_description/__init__.py b/src/labthings_fastapi/thing_description/__init__.py index a06497f1..5691c838 100644 --- a/src/labthings_fastapi/thing_description/__init__.py +++ b/src/labthings_fastapi/thing_description/__init__.py @@ -278,7 +278,7 @@ def jsonschema_to_dataschema( return output -def type_to_dataschema(t: type, **kwargs) -> DataSchema: +def type_to_dataschema(t: type, **kwargs: Any) -> DataSchema: r"""Convert a Python type to a Thing Description DataSchema. This makes use of pydantic's `schema_of` function to create a diff --git a/src/labthings_fastapi/thing_description/_model.py b/src/labthings_fastapi/thing_description/_model.py index f5bdcf9a..8c3c1633 100644 --- a/src/labthings_fastapi/thing_description/_model.py +++ b/src/labthings_fastapi/thing_description/_model.py @@ -81,7 +81,7 @@ class Subprotocol(Enum): ] -def uses_thing_context(v: ThingContextType): +def uses_thing_context(v: ThingContextType) -> None: """Check the URLs in the ThingContextType are valid. This function makes ``assert`` statements, so will fail with an exception diff --git a/src/labthings_fastapi/types/numpy.py b/src/labthings_fastapi/types/numpy.py index e8d067fb..2fb27db0 100644 --- a/src/labthings_fastapi/types/numpy.py +++ b/src/labthings_fastapi/types/numpy.py @@ -71,7 +71,7 @@ def np_to_listoflists(arr: np.ndarray) -> NestedListOfNumbers: :param arr: a `numpy.ndarray`. :return: a nested list of numbers. """ - return arr.tolist() # type: ignore[return-value] + return arr.tolist() def listoflists_to_np(lol: Union[NestedListOfNumbers, np.ndarray]) -> np.ndarray: diff --git a/src/labthings_fastapi/utilities/introspection.py b/src/labthings_fastapi/utilities/introspection.py index 8cbeaf6a..f991f33e 100644 --- a/src/labthings_fastapi/utilities/introspection.py +++ b/src/labthings_fastapi/utilities/introspection.py @@ -114,9 +114,7 @@ def input_model_from_signature( p_type = Any if p.annotation is Parameter.empty else type_hints[name] # pydantic uses `...` to represent missing defaults (i.e. required params) default = Field(...) if p.default is Parameter.empty else p.default - # p_type below has a complicated type, but it is reasonable to - # call p_type a `type` and ignore the mypy error. - fields[name] = (p_type, default) # type: ignore[assignment] + fields[name] = (p_type, default) model = create_model( # type: ignore[call-overload] f"{func.__name__}_input", model_config=ConfigDict(extra="allow" if takes_v_kwargs else "forbid"), @@ -176,7 +174,7 @@ def return_type(func: Callable) -> Type: """ sig = inspect.signature(func) if sig.return_annotation == inspect.Signature.empty: - return Any # type: ignore[return-value] + return Any else: # We use `get_type_hints` rather than just `sig.return_annotation` # because it resolves forward references, etc. diff --git a/tests/test_property.py b/tests/test_property.py index aad5db5b..2ffac7f9 100644 --- a/tests/test_property.py +++ b/tests/test_property.py @@ -220,6 +220,20 @@ class Example: assert set(entry.keys()) == set(["get", "put"]) +def test_baseproperty_set_error(): + """Check `.Baseproperty.__set__` raises an error and is overridden.""" + assert tp.DataProperty.__get__ is tp.BaseProperty.__get__ + assert tp.DataProperty.__set__ is not tp.BaseProperty.__set__ + assert tp.FunctionalProperty.__set__ is not tp.BaseProperty.__set__ + + class Example: + bp: int = tp.BaseProperty() + + example = Example() + with pytest.raises(NotImplementedError): + example.bp = 0 + + def test_decorator_exception(): r"""Check decorators work as expected when the setter has a different name. diff --git a/typing_tests/thing_definitions.py b/typing_tests/thing_definitions.py index b0815e91..5650f2f1 100644 --- a/typing_tests/thing_definitions.py +++ b/typing_tests/thing_definitions.py @@ -59,7 +59,7 @@ def int_factory() -> int: @lt.property -def strprop(self) -> str: +def strprop(self: typing.Any) -> str: """A functional property that should not cause mypy errors.""" return "foo" @@ -213,7 +213,7 @@ def intprop2(self) -> int: return 0 @intprop2.setter - def set_intprop2(self, value: int): + def set_intprop2(self, value: int) -> None: """Setter for intprop2.""" pass