[RSDK-878] Typechecking (#512)

Author: njooma

.github/workflows/test.yml

@@ -9,7 +9,7 @@ on:
   push:
     branches: [main]
   pull_request_target:
-    branches: [main, 'rc-*']
+    branches: [main, "rc-*"]
 
 jobs:
   test:
@@ -44,6 +44,9 @@ jobs:
         run: poetry run pip install -r requirements-test.txt
         if: ${{ matrix.requirements-version == 'min' }}
 
+      - name: Type Check
+        run: make typecheck
+
       - name: Lint
         run: make lint
 

Makefile

@@ -14,6 +14,12 @@ _format:
 format:
 	poetry run $(MAKE) _format
 
+_typecheck:
+	pyright
+
+typecheck:
+	poetry run $(MAKE) _typecheck
+
 _buf: clean
 	rm -rf src/viam/gen
 	buf generate buf.build/viamrobotics/api

poetry.lock

@@ -23,29 +23,34 @@ include = ["LICENSE", "src/viam/rpc/libviam_rust_utils.*"]
   numpy = { version = ">=1.21,<2", optional = true }
 
   [tool.poetry.group.dev.dependencies]
-  pytest = "^7.4.2"
-  pytest-asyncio = "^0.21.1"
+  pytest = "^7.4.3"
+  pytest-asyncio = "^0.23.2"
   coverage = "^7.3.2"
   protoletariat = "^3.2.19"
   jupyter = "^1.0.0"
   flake8 = "^6.1.0"
-  myst-nb = "^0.17.2"
-  sphinx-autoapi = "^2.1.1"
-  sphinx-rtd-theme = "^1.3.0"
+  myst-nb = [
+    {version = "<1.0.0", python = "<3.9"},
+    {version = ">=1.0.0", python = ">=3.9"},
+  ]
+  sphinx-autoapi = [
+    {version = "<3.0.0", python = "<3.9"},
+    {version = ">=3.0.0", python = ">=3.9"},
+  ]
+  sphinx-rtd-theme = "^2.0.0"
   autopep8 = "^2.0.4"
   black = {extras = ["jupyter"], version = "^23.12.0"}
-  nbmake = "^1.4.5"
-  types-pillow = "^10.0.0.3"
+  nbmake = "^1.4.6"
+  types-pillow = "^10.1.0.2"
   mypy-protobuf = "^3.5.0"
-  tox = "^4.11.3"
+  tox = "^4.11.4"
   isort = "^5.12.0"
   pytest-watcher = "^0.3.4"
-  # TODO(RSDK-5450) - Update myst-nb so this can be unpinned.
-  astroid = "<=2.15.6"
   numpy = [
     {version = "<1.25.0", python = "<3.9"},
-    {version = ">=1.25.0", python = ">=3.9"},
+    {version = ">=1.26.2", python = ">=3.9"},
   ]
+  pyright = "^1.1.339"
 
 [tool.pytest.ini_options]
 addopts = "-ra"
@@ -72,3 +77,7 @@ build-backend = "poetry.core.masonry.api"
 
 [tool.poetry.extras]
 mlmodel = ["numpy"]
+
+[tool.pyright]
+include = [ "src" ]
+exclude = [ "**/gen", "**/proto" ]

pyproject.toml

@@ -41,6 +41,22 @@ def _log_exceptions(exctype, value, traceback):
 ##################
 # MONKEY PATCHES #
 ##################
-_ResourceName.__str__ = lambda self: f"{self.namespace}:{self.type}:{self.subtype}/{self.name}"
-_ResourceName.__repr__ = lambda self: f"<viam.proto.common.ResourceName {str(self)} at {hex(id(self))}>"
-_ResourceName.__hash__ = lambda self: hash(str(self))
+def _rname_str(self: _ResourceName) -> str:
+    return f"{self.namespace}:{self.type}:{self.subtype}/{self.name}"
+
+
+_ResourceName.__str__ = _rname_str
+
+
+def _rname_repr(self: _ResourceName) -> str:
+    return f"<viam.proto.common.ResourceName {str(self)} at {hex(id(self))}>"
+
+
+_ResourceName.__repr__ = _rname_repr
+
+
+def _rname_hash(self: _ResourceName) -> int:
+    return hash(str(self))
+
+
+_ResourceName.__hash__ = _rname_hash  # type: ignore

src/viam/__init__.py

@@ -17,11 +17,11 @@ async def next(self) -> LogsType:
     async def close(self):
         ...
 
-    def __aiter__(self):
-        return self
+    def __aiter__(self) -> AsyncIterator:
+        ...
 
     async def __anext__(self) -> LogsType:
-        return await self.next()
+        ...
 
 
 class _LogsStreamWithIterator(_LogsStream[LogsType]):

src/viam/app/_logs.py

@@ -434,8 +434,8 @@ async def _create_authorization_for_new_api_key(self, auth: APIKeyAuthorization)
         return await self._create_authorization(
             identity_id="",  # setting `identity_id` when creating an API key results in an error
             identity_type="api-key",
-            role=auth._role,
-            resource_type=auth._resource_type,
+            role=auth._role,  # type: ignore -- Ignoring because this is technically a `string`
+            resource_type=auth._resource_type,  # type: ignore -- Ignoring because this is technically a `string`
             resource_id=auth._resource_id,
         )
 
@@ -1282,7 +1282,7 @@ async def list_authorizations(self, resource_ids: Optional[List[str]] = None) ->
         response: ListAuthorizationsResponse = await self._app_client.ListAuthorizations(request, metadata=self._metadata)
         return list(response.authorizations)
 
-    async def check_permissions(self, permissions: [List[AuthorizedPermissions]]) -> List[AuthorizedPermissions]:
+    async def check_permissions(self, permissions: List[AuthorizedPermissions]) -> List[AuthorizedPermissions]:
         """Checks validity of a list of permissions.
 
         Args:
@@ -1369,9 +1369,10 @@ async def upload_module_file(self, module_file_info: Optional[ModuleFileInfo], f
         async with self._app_client.UploadModuleFile.open(metadata=self._metadata) as stream:
             await stream.send_message(request_module_file_info)
             await stream.send_message(request_file, end=True)
-            response = await stream.recv_message()
+            response: Union[UploadModuleFileRequest, None] = await stream.recv_message()
             if not response:
                 await stream.recv_trailing_metadata()  # causes us to throw appropriate gRPC error.
+                raise TypeError("Response cannot be empty")  # we should never get here, but for typechecking
             return response.url
 
     async def get_module(self, module_id: str) -> Module:
@@ -1418,8 +1419,8 @@ async def create_key(self, authorizations: List[APIKeyAuthorization], name: Opti
             Tuple[str, str]: The api key and api key ID.
         """
         name = name if name is not None else str(datetime.now())
-        authorizationspb = [await self._create_authorization_for_new_api_key(auth) for auth in authorizations]
-        request = CreateKeyRequest(authorizations=authorizationspb, name=name)
+        authorizations_pb = [await self._create_authorization_for_new_api_key(auth) for auth in authorizations]
+        request = CreateKeyRequest(authorizations=authorizations_pb, name=name)
         response: CreateKeyResponse = await self._app_client.CreateKey(request, metadata=self._metadata)
         return (response.key, response.id)
 
@@ -1447,4 +1448,4 @@ async def list_keys(self) -> List[APIKeyWithAuthorizations]:
         org_id = await self._get_organization_id()
         request = ListKeysRequest(org_id=org_id)
         response: ListKeysResponse = await self._app_client.ListKeys(request, metadata=self._metadata)
-        return [key for key in response.api_keys]
+        return list(response.api_keys)

src/viam/app/app_client.py

@@ -1,6 +1,6 @@
 from typing import Mapping, Optional
 
-from grpclib.client import Channel
+from grpclib.client import Channel, Stream
 
 from viam import logging
 from viam.proto.app.billing import (
@@ -60,11 +60,12 @@ async def get_invoice_pdf(self, invoice_id: str, org_id: str, dest: str, timeout
             org_id (str): the ID of the org to request data from
             dest (str): filepath to save the invoice to
         """
-        request = GetInvoicePdfRequest(id=invoice_id, org_id=org_id)
-        response: GetInvoicePdfResponse = await self._billing_client.GetInvoicePdf(request, metadata=self._metadata, timeout=timeout)
-        data: bytes = response[0].chunk
-        with open(dest, "wb") as file:
-            file.write(data)
+        stream: Stream[GetInvoicePdfRequest, GetInvoicePdfResponse]
+        async with self._billing_client.GetInvoicePdf.open(timeout=timeout, metadata=self._metadata) as stream:
+            await stream.send_message(GetInvoicePdfRequest(id=invoice_id, org_id=org_id), end=True)
+            with open(dest, "wb") as file:
+                async for response in stream:
+                    file.write(response.chunk)
 
     async def get_invoices_summary(self, org_id: str, timeout: Optional[float] = None) -> GetInvoicesSummaryResponse:
         """Access total outstanding balance plus invoice summaries for a given org.

src/viam/app/billing_client.py

@@ -93,8 +93,10 @@ def __init__(self, data: Mapping[str, Any], metadata: CaptureMetadata, time_requ
         def __str__(self) -> str:
             return f"{self.data}\n{self.metadata}Time requested: {self.time_requested}\nTime received: {self.time_received}\n"
 
-        def __eq__(self, other: "DataClient.TabularData") -> bool:
-            return str(self) == str(other)
+        def __eq__(self, other: object) -> bool:
+            if isinstance(other, DataClient.TabularData):
+                return str(self) == str(other)
+            return False
 
     class BinaryData:
         """Class representing a piece of binary data and associated metadata.
@@ -114,8 +116,10 @@ def __init__(self, data: bytes, metadata: BinaryMetadata) -> None:
         def __str__(self) -> str:
             return f"{self.data}\n{self.metadata}"
 
-        def __eq__(self, other: "DataClient.BinaryData") -> bool:
-            return str(self) == str(other)
+        def __eq__(self, other: object) -> bool:
+            if isinstance(other, DataClient.BinaryData):
+                return str(self) == str(other)
+            return False
 
     def __init__(self, channel: Channel, metadata: Mapping[str, str]):
         """Create a `DataClient` that maintains a connection to app.

src/viam/app/data_client.py

@@ -16,7 +16,7 @@ class Arm(ComponentBase):
     overridden, it must call the ``super().__init__()`` function.
     """
 
-    SUBTYPE: Final = Subtype(RESOURCE_NAMESPACE_RDK, RESOURCE_TYPE_COMPONENT, "arm")
+    SUBTYPE: Final = Subtype(RESOURCE_NAMESPACE_RDK, RESOURCE_TYPE_COMPONENT, "arm")  # pyright: ignore [reportIncompatibleVariableOverride]
 
     @abc.abstractmethod
     async def get_end_position(