[WIP] Enable mypy

.pre-commit-config.yaml

@@ -51,14 +51,14 @@ repos:
     hooks:
       - id: prettier
 
-  # todo - re-enable mypy & fixes in a separate PR
-  # - repo: https://github.com/pre-commit/mirrors-mypy
-  #   rev: v1.10.0
-  #   hooks:
-  #     - id: mypy
-  #       name: "mypy"
-  #       always_run: true
-  #       files: ^syftbox
+  - repo: https://github.com/pre-commit/mirrors-mypy
+    rev: v1.10.0
+    hooks:
+      - id: mypy
+        name: "mypy"
+        always_run: true
+        files: ^syftbox
+        args: ["--config-file=tox.ini", "--install-types", "--non-interactive"]
 
   # - repo: meta
   #   hooks:

notebooks/02-trade-code.ipynb

@@ -176,9 +176,7 @@
    "source": [
     "# TEMP bug where we cant use theirs_with_my_read because the parent write is ignored but allowing the perm file to set its own\n",
     "# rules wont work either so we need to solve the permissioning of files themselves\n",
-    "path = myanalysis.to_flow(\n",
-    "    client_config=client_config, inputs={\"trade_data\": trade_data}\n",
-    ")\n",
+    "path = myanalysis.to_flow(client_config=client_config, inputs={\"trade_data\": trade_data})\n",
     "path"
    ]
   },

notebooks/03-netflix-code.ipynb

@@ -347,15 +347,10 @@
     "        non_zero_indices = np.nonzero(tv_count_array)[0].astype(int)\n",
     "        non_zero_values = tv_count_array[non_zero_indices].astype(int)\n",
     "        outer_part = counter * slice_size\n",
-    "        non_zero_dict = {\n",
-    "            int(k + outer_part): int(v)\n",
-    "            for k, v in dict(zip(non_zero_indices, non_zero_values)).items()\n",
-    "        }\n",
+    "        non_zero_dict = {int(k + outer_part): int(v) for k, v in dict(zip(non_zero_indices, non_zero_values)).items()}\n",
     "        tmdb_id_value_counts.update(non_zero_dict)\n",
     "        counter += 1\n",
-    "        stats[\"value_counts\"] = dict(\n",
-    "            sorted(tmdb_id_value_counts.items(), key=lambda item: item[1], reverse=True)\n",
-    "        )\n",
+    "        stats[\"value_counts\"] = dict(sorted(tmdb_id_value_counts.items(), key=lambda item: item[1], reverse=True))\n",
     "    return stats\n",
     "\n",
     "\n",
@@ -380,13 +375,9 @@
     "    top_5_summary = {}\n",
     "    top_5_summary[\"avg_time\"] = round(all_results[\"total_time\"] / num_parties)\n",
     "    top_5_summary[\"avg_views\"] = round(all_results[\"total_views\"] / num_parties)\n",
-    "    top_5_summary[\"avg_unique_show_views\"] = round(\n",
-    "        all_results[\"total_unique_show_views\"] / num_parties\n",
-    "    )\n",
+    "    top_5_summary[\"avg_unique_show_views\"] = round(all_results[\"total_unique_show_views\"] / num_parties)\n",
     "    top_5_summary[\"top_5\"] = dict(\n",
-    "        sorted(\n",
-    "            all_results[\"value_counts\"].items(), key=lambda item: item[1], reverse=True\n",
-    "        )[:top_k]\n",
+    "        sorted(all_results[\"value_counts\"].items(), key=lambda item: item[1], reverse=True)[:top_k]\n",
     "    )\n",
     "    return top_5_summary\n",
     "\n",

syftbox/app/cli.py

@@ -37,7 +37,7 @@
 
 
 @app.command()
-def list(config_path: Annotated[Path, CONFIG_OPTS] = DEFAULT_CONFIG_PATH):
+def list(config_path: Annotated[Path, CONFIG_OPTS] = DEFAULT_CONFIG_PATH) -> None:
     """List all installed Syftbox apps"""
     workspace = get_workspace(config_path)
     result = list_app(workspace)
@@ -57,7 +57,7 @@ def install(
     branch: Annotated[str, BRANCH_OPTS] = "main",
     config_path: Annotated[Path, CONFIG_OPTS] = DEFAULT_CONFIG_PATH,
     called_by: Annotated[str, CALLED_BY_OPTS] = "user",
-):
+) -> None:
     """Install a new Syftbox app"""
     client = get_client(config_path)
     result = install_app(client.workspace, repository, branch)
@@ -77,7 +77,7 @@ def install(
 def uninstall(
     app_name: Annotated[str, UNINSTALL_ARGS],
     config_path: Annotated[Path, CONFIG_OPTS] = DEFAULT_CONFIG_PATH,
-):
+) -> None:
     """Uninstall a Syftbox app"""
     workspace = get_workspace(config_path)
     result = uninstall_app(app_name, workspace)
@@ -92,11 +92,11 @@ def uninstall(
 def run(
     app_name: str,
     config_path: Annotated[Path, CONFIG_OPTS] = DEFAULT_CONFIG_PATH,
-):
+) -> None:
     """Run a Syftbox app"""
     workspace = get_workspace(config_path)
 
-    extra_args = []
+    extra_args: list = []
     try:
         rprint(f"Running [bold]'{app_name}'[/bold]\nLocation: '{workspace.apps}'\n")
         result = find_and_run_script(str(workspace.apps / app_name), extra_args, str(config_path))
@@ -110,7 +110,7 @@ def run(
 
 
 @app.command(rich_help_panel="General Options")
-def env(with_syftbox: bool = False):
+def env(with_syftbox: bool = False) -> None:
     """Setup virtual env for app. With option to install syftbox matching client version"""
 
     script = APP_ENV_SCRIPT

syftbox/client/api.py

@@ -2,14 +2,16 @@
 
 from fastapi import FastAPI, Request
 from fastapi.middleware.cors import CORSMiddleware
-from starlette.middleware.base import BaseHTTPMiddleware
+from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
+from starlette.responses import Response
+from typing_extensions import AsyncGenerator
 
 from syftbox.client.base import SyftClientInterface
 from syftbox.client.routers import app_router, datasite_router, index_router
 
 
 class NoCacheMiddleware(BaseHTTPMiddleware):
-    async def dispatch(self, request: Request, call_next):
+    async def dispatch(self, request: Request, call_next: RequestResponseEndpoint) -> Response:
         response = await call_next(request)
         response.headers["Cache-Control"] = "no-cache, no-store, must-revalidate"
         response.headers["Pragma"] = "no-cache"
@@ -18,7 +20,7 @@ async def dispatch(self, request: Request, call_next):
 
 
 @contextlib.asynccontextmanager
-async def lifespan(app: FastAPI):
+async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
     yield
 
 

syftbox/client/cli.py

@@ -80,13 +80,13 @@
 def client(
     ctx: Context,
     data_dir: Annotated[Path, DATA_DIR_OPTS] = DEFAULT_DATA_DIR,
-    email: Annotated[str, EMAIL_OPTS] = None,
+    email: Annotated[str, EMAIL_OPTS] = "",
     server: Annotated[str, SERVER_OPTS] = DEFAULT_SERVER_URL,
     config_path: Annotated[Path, CONFIG_OPTS] = DEFAULT_CONFIG_PATH,
     port: Annotated[int, PORT_OPTS] = DEFAULT_PORT,
     open_dir: Annotated[bool, OPEN_OPTS] = True,
     verbose: Annotated[bool, VERBOSE_OPTS] = False,
-):
+) -> None:
     """Run the SyftBox client"""
 
     if ctx.invoked_subcommand is not None:
@@ -121,7 +121,7 @@ def client(
 def report(
     output_path: Annotated[Path, REPORT_PATH_OPTS] = Path(".").resolve(),
     config_path: Annotated[Path, CONFIG_OPTS] = DEFAULT_CONFIG_PATH,
-):
+) -> None:
     """Generate a report of the SyftBox client"""
     from datetime import datetime
 
@@ -139,7 +139,7 @@ def report(
         raise Exit(1)
 
 
-def main():
+def main() -> None:
     app()
 
 

syftbox/client/cli_setup.py

@@ -8,6 +8,7 @@
 
 from rich import print as rprint
 from rich.prompt import Confirm, Prompt
+from typing_extensions import Optional
 
 from syftbox.__version__ import __version__
 from syftbox.client.auth import authenticate_user
@@ -42,7 +43,7 @@ def prompt_delete_old_data_dir(data_dir: Path) -> bool:
     return Confirm.ask(msg)
 
 
-def get_migration_decision(data_dir: Path):
+def get_migration_decision(data_dir: Path) -> bool:
     migrate_datasite = False
     if data_dir.exists():
         if is_empty(data_dir):
@@ -66,7 +67,7 @@ def setup_config_interactive(
     """Setup the client configuration interactively. Called from CLI"""
 
     config_path = config_path.expanduser().resolve()
-    conf: SyftClientConfig = None
+    conf: Optional[SyftClientConfig] = None
     if data_dir:
         data_dir = data_dir.expanduser().resolve()
 

syftbox/client/client2.py

@@ -9,6 +9,7 @@
 import uvicorn
 from loguru import logger
 from pid import PidFile, PidFileAlreadyLockedError, PidFileAlreadyRunningError
+from typing_extensions import Optional
 
 from syftbox.__version__ import __version__
 from syftbox.client.api import create_api
@@ -47,7 +48,7 @@ class SyftClient:
         Exception: If the client fails to start due to any reason
     """
 
-    def __init__(self, config: SyftClientConfig, log_level: str = "INFO", **kwargs):
+    def __init__(self, config: SyftClientConfig, log_level: str = "INFO", **kwargs: dict) -> None:
         self.config = config
         self.log_level = log_level
 
@@ -61,12 +62,12 @@ def __init__(self, config: SyftClientConfig, log_level: str = "INFO", **kwargs):
 
         # kwargs for making customization/unit testing easier
         # this will be replaced with a sophisticated plugin system
-        self.__sync_manager: SyncManager = kwargs.get("sync_manager", None)
-        self.__app_runner: AppRunner = kwargs.get("app_runner", None)
+        self.__sync_manager: Optional[SyncManager] = kwargs.get("sync_manager", None)  # type: ignore
+        self.__app_runner: Optional[AppRunner] = kwargs.get("app_runner", None)  # type: ignore
         self.__local_server: uvicorn.Server = None
 
     @property
-    def sync_manager(self):
+    def sync_manager(self) -> SyncManager:
         """the sync manager. lazily initialized"""
         if self.__sync_manager is None:
             try:
@@ -76,7 +77,7 @@ def sync_manager(self):
         return self.__sync_manager
 
     @property
-    def app_runner(self):
+    def app_runner(self) -> AppRunner:
         """the app runner. lazily initialized"""
         if self.__app_runner is None:
             try:
@@ -100,7 +101,7 @@ def public_dir(self) -> Path:
         """The public directory in the datasite of the current user"""
         return self.datasite / "public"
 
-    def start(self):
+    def start(self) -> None:
         try:
             self.pid.create()
         except PidFileAlreadyLockedError:
@@ -128,7 +129,7 @@ def create_metadata_file(self) -> None:
         metadata_json["version"] = __version__
         self.metadata_path.write_text(json.dumps(metadata_json, indent=2))
 
-    def shutdown(self):
+    def shutdown(self) -> None:
         if self.__local_server:
             _result = asyncio.run(self.__local_server.shutdown())
 
@@ -149,12 +150,12 @@ def check_pidfile(self) -> str:
         except PidFileAlreadyRunningError:
             raise SyftBoxAlreadyRunning(f"Another instance of SyftBox is running on {self.config.data_dir}")
 
-    def init_datasite(self):
+    def init_datasite(self) -> None:
         if self.datasite.exists():
             return
         create_datasite(self.workspace.datasites, self.config.email)
 
-    def register_self(self):
+    def register_self(self) -> None:
         """Register the user's email with the SyftBox cache server"""
         if self.is_registered:
             return
@@ -174,7 +175,7 @@ def as_context(self) -> "SyftClientContext":
         """Return a implementation of SyftClientInterface to be injected into sub-systems"""
         return SyftClientContext(self.config, self.workspace, self.server_client)
 
-    def __run_local_server(self):
+    def __run_local_server(self) -> None:
         logger.info(f"Starting local server on {self.config.client_url}")
         app = create_api(self.as_context())
         self.__local_server = uvicorn.Server(
@@ -193,17 +194,17 @@ def __register_email(self) -> str:
         response.raise_for_status()
         return response.json().get("token")
 
-    def __enter__(self):
+    def __enter__(self) -> "SyftClient":
         return self
 
-    def __exit__(self, exc_type, exc_val, exc_tb):
+    def __exit__(self) -> None:
         self.shutdown()
 
     # utils
-    def open_datasites_dir(self):
+    def open_datasites_dir(self) -> None:
         file_manager.open_dir(self.workspace.datasites)
 
-    def copy_icons(self):
+    def copy_icons(self) -> None:
         self.workspace.mkdirs()
         if platform.system() == "Darwin":
             macos.copy_icon_file(ICON_FOLDER, self.workspace.data_dir)
@@ -246,7 +247,7 @@ def all_datasites(self) -> list[str]:
     def __repr__(self) -> str:
         return f"SyftClientContext<{self.config.email}, {self.config.data_dir}>"
 
-    def log_analytics_event(self, event_name: str, **kwargs) -> None:
+    def log_analytics_event(self, event_name: str, **kwargs: dict) -> None:
         """Log an event to the server"""
         event_data = {
             "event_name": event_name,
@@ -258,7 +259,7 @@ def log_analytics_event(self, event_name: str, **kwargs) -> None:
             raise SyftServerError(f"Failed to log event: {response.text}")
 
 
-def run_apps_to_api_migration(new_ws: SyftWorkspace):
+def run_apps_to_api_migration(new_ws: SyftWorkspace) -> None:
     old_sync_folder = new_ws.data_dir
     old_apps_dir = old_sync_folder / "apps"
     new_apps_dir = new_ws.apps
@@ -270,7 +271,7 @@ def run_apps_to_api_migration(new_ws: SyftWorkspace):
         shutil.move(str(old_apps_dir), str(new_apps_dir))
 
 
-def run_migration(config: SyftClientConfig, migrate_datasite=True):
+def run_migration(config: SyftClientConfig, migrate_datasite: bool = True) -> None:
     # first run config migration
     config.migrate()
 
@@ -311,7 +312,7 @@ def run_migration(config: SyftClientConfig, migrate_datasite=True):
 
 
 def run_client(
-    client_config: SyftClientConfig, open_dir: bool = False, log_level: str = "INFO", migrate_datasite=True
+    client_config: SyftClientConfig, open_dir: bool = False, log_level: str = "INFO", migrate_datasite: bool = True
 ) -> int:
     """Run the SyftBox client"""
     client = None
@@ -329,9 +330,12 @@ def run_client(
     try:
         client = SyftClient(client_config, log_level=log_level)
         # we don't want to run migration if another instance of client is already running
-        bool(client.check_pidfile()) and run_migration(client_config, migrate_datasite=migrate_datasite)
-        (not syftbox_env.DISABLE_ICONS) and client.copy_icons()
-        open_dir and client.open_datasites_dir()
+        if client.check_pidfile():
+            run_migration(client_config, migrate_datasite=migrate_datasite)
+        if not syftbox_env.DISABLE_ICONS:
+            client.copy_icons()
+        if open_dir:
+            client.open_datasites_dir()
         client.start()
     except SyftBoxAlreadyRunning as e:
         logger.error(e)
@@ -342,5 +346,6 @@ def run_client(
         logger.exception("Unhandled exception when starting the client", e)
         return -2
     finally:
-        client and client.shutdown()
+        if client is not None:
+            client.shutdown()
     return 0

syftbox/client/fsevents.py

@@ -20,7 +20,7 @@ def __init__(self, watch_dir: Path, event_handler: FileSystemEventHandler):
         self.event_handler = event_handler
         self._observer = Observer()
 
-    def start(self):
+    def start(self) -> None:
         # observer starts it's own thread
         self._observer.schedule(
             self.event_handler,
@@ -29,7 +29,7 @@ def start(self):
         )
         self._observer.start()
 
-    def stop(self):
+    def stop(self) -> None:
         self._observer.stop()
         self._observer.join()