mypy static type checking for mlos_bench

.vscode/settings.json

@@ -1,7 +1,10 @@
 // vim: set ft=jsonc:
 {
     "makefile.extensionOutputFolder": "./.vscode",
-    "python.defaultInterpreterPath": "${env:HOME}${env:USERPROFILE}/.conda/envs/mlos_core/bin/python",
+    // Note: this only works in WSL/Linux currently.
+    "python.defaultInterpreterPath": "${env:HOME}/.conda/envs/mlos_core/bin/python",
+    // For Windows it should be this instead:
+    //"python.defaultInterpreterPath": "${env:USERPROFILE}/.conda/envs/mlos_core/python.exe",
     "python.testing.pytestEnabled": true,
     "python.linting.enabled": true,
     "python.linting.pylintEnabled": true,

Makefile

@@ -12,7 +12,7 @@ MLOS_BENCH_PYTHON_FILES := $(shell find ./mlos_bench/ -type f -name '*.py' 2>/de
 
 DOCKER := $(shell which docker)
 # Make sure the build directory exists.
-MKDIR_BUILD := $(shell mkdir -p build)
+MKDIR_BUILD := $(shell test -d build || mkdir build)
 
 # Allow overriding the default verbosity of conda for CI jobs.
 #CONDA_INFO_LEVEL ?= -q
@@ -113,7 +113,7 @@ build/pylint.%.${CONDA_ENV_NAME}.build-stamp: build/conda-env.${CONDA_ENV_NAME}.
 	touch $@
 
 .PHONY: mypy
-mypy: conda-env build/mypy.mlos_core.${CONDA_ENV_NAME}.build-stamp # TODO: build/mypy.mlos_bench.${CONDA_ENV_NAME}.build-stamp
+mypy: conda-env build/mypy.mlos_core.${CONDA_ENV_NAME}.build-stamp build/mypy.mlos_bench.${CONDA_ENV_NAME}.build-stamp
 
 build/mypy.mlos_core.${CONDA_ENV_NAME}.build-stamp: $(MLOS_CORE_PYTHON_FILES)
 build/mypy.mlos_bench.${CONDA_ENV_NAME}.build-stamp: $(MLOS_BENCH_PYTHON_FILES)
@@ -339,6 +339,7 @@ build/check-doc.build-stamp: doc/build/html/index.html doc/build/html/htmlcov/in
 			-e 'Problems with "include" directive path:' \
 			-e 'duplicate object description' \
 			-e "document isn't included in any toctree" \
+			-e "more than one target found for cross-reference" \
 			-e "toctree contains reference to nonexisting document 'auto_examples/index'" \
 			-e "failed to import function 'create' from module '(SpaceAdapter|Optimizer)Factory'" \
 			-e "No module named '(SpaceAdapter|Optimizer)Factory'" \

doc/Dockerfile

@@ -1,6 +1,6 @@
 FROM nginx:latest
 RUN apt-get update && apt-get install -y --no-install-recommends linklint curl
-# Our ngxinx config overrides the default listening port.
+# Our nginx config overrides the default listening port.
 ARG NGINX_PORT=81
 ENV NGINX_PORT=${NGINX_PORT}
 EXPOSE ${NGINX_PORT}

doc/source/overview.rst

@@ -156,6 +156,12 @@ Service Mix-ins
 
    Service
    FileShareService
+
+.. currentmodule:: mlos_bench.service.config_persistence
+.. autosummary::
+   :toctree: generated/
+   :template: class.rst
+
    ConfigPersistenceService
 
 Local Services

mlos_bench/config/applications/generate_redis_config.py

@@ -13,7 +13,7 @@
 import argparse
 
 
-def _main(fname_input: str, fname_output: str):
+def _main(fname_input: str, fname_output: str) -> None:
     with open(fname_input, "rt", encoding="utf-8") as fh_tunables, \
          open(fname_output, "wt", encoding="utf-8", newline="") as fh_config:
         for (key, val) in json.load(fh_tunables).items():

mlos_bench/config/applications/process_redis_results.py

@@ -12,7 +12,7 @@
 import pandas as pd
 
 
-def _main(input_file: str, output_file: str):
+def _main(input_file: str, output_file: str) -> None:
     """
     Re-shape Redis benchmark CSV results from wide to long.
     """

mlos_bench/config/linux-boot/generate_grub_config.py

@@ -13,7 +13,7 @@
 import argparse
 
 
-def _main(fname_input: str, fname_output: str):
+def _main(fname_input: str, fname_output: str) -> None:
     with open(fname_input, "rt", encoding="utf-8") as fh_tunables, \
          open(fname_output, "wt", encoding="utf-8", newline="") as fh_config:
         for (key, val) in json.load(fh_tunables).items():

mlos_bench/config/linux-setup/generate_kernel_config_script.py

@@ -13,7 +13,7 @@
 import argparse
 
 
-def _main(fname_input: str, fname_output: str):
+def _main(fname_input: str, fname_output: str) -> None:
     with open(fname_input, "rt", encoding="utf-8") as fh_tunables, \
          open(fname_output, "wt", encoding="utf-8", newline="") as fh_config:
         for (key, val) in json.load(fh_tunables).items():

mlos_bench/mlos_bench/environment/base_environment.py

@@ -9,22 +9,33 @@
 import abc
 import json
 import logging
-from typing import Optional, Tuple
+from typing import Dict, Optional, Tuple
 
 from mlos_bench.environment.status import Status
+from mlos_bench.service.base_service import Service
+from mlos_bench.service.types.config_loader_type import SupportsConfigLoading
+from mlos_bench.tunables.tunable import TunableValue
 from mlos_bench.tunables.tunable_groups import TunableGroups
 from mlos_bench.util import instantiate_from_config
 
 _LOG = logging.getLogger(__name__)
 
 
 class Environment(metaclass=abc.ABCMeta):
+    # pylint: disable=too-many-instance-attributes
     """
     An abstract base of all benchmark environments.
     """
 
     @classmethod
-    def new(cls, env_name, class_name, config, global_config=None, tunables=None, service=None):
+    def new(cls,
+            env_name: str,
+            class_name: str,
+            config: dict,
+            global_config: Optional[dict] = None,
+            tunables: Optional[TunableGroups] = None,
+            service: Optional[Service] = None,
+            ) -> "Environment":
         # pylint: disable=too-many-arguments
         """
         Factory method for a new environment with a given config.
@@ -58,7 +69,12 @@ def new(cls, env_name, class_name, config, global_config=None, tunables=None, se
         return instantiate_from_config(cls, class_name, env_name, config,
                                        global_config, tunables, service)
 
-    def __init__(self, name, config, global_config=None, tunables=None, service=None):
+    def __init__(self,
+                 name: str,
+                 config: dict,
+                 global_config: Optional[dict] = None,
+                 tunables: Optional[TunableGroups] = None,
+                 service: Optional[Service] = None):
         # pylint: disable=too-many-arguments
         """
         Create a new environment with a given config.
@@ -84,10 +100,17 @@ def __init__(self, name, config, global_config=None, tunables=None, service=None
         self.config = config
         self._service = service
         self._is_ready = False
-        self._params = {}
+        self._params: Dict[str, TunableValue] = {}
+
+        self._config_loader_service: SupportsConfigLoading
+        if self._service is not None and isinstance(self._service, SupportsConfigLoading):
+            self._config_loader_service = self._service
+
+        if global_config is None:
+            global_config = {}
 
         self._const_args = config.get("const_args", {})
-        for key in set(self._const_args).intersection(global_config or {}):
+        for key in set(self._const_args).intersection(global_config):
             self._const_args[key] = global_config[key]
 
         for key in config.get("required_args", []):
@@ -110,13 +133,13 @@ def __init__(self, name, config, global_config=None, tunables=None, service=None
             _LOG.debug("Config for: %s\n%s",
                        name, json.dumps(self.config, indent=2))
 
-    def __str__(self):
+    def __str__(self) -> str:
         return self.name
 
-    def __repr__(self):
+    def __repr__(self) -> str:
         return f"Env: {self.__class__} :: '{self.name}'"
 
-    def _combine_tunables(self, tunables):
+    def _combine_tunables(self, tunables: TunableGroups) -> Dict[str, TunableValue]:
         """
         Plug tunable values into the base config. If the tunable group is unknown,
         ignore it (it might belong to another environment). This method should
@@ -130,14 +153,14 @@ def _combine_tunables(self, tunables):
 
         Returns
         -------
-        params : dict
+        params : Dict[str, Union[int, float, str]]
             Free-format dictionary that contains the new environment configuration.
         """
         return tunables.get_param_values(
-            group_names=self._tunable_params.get_names(),
+            group_names=list(self._tunable_params.get_names()),
             into_params=self._const_args.copy())
 
-    def tunable_params(self):
+    def tunable_params(self) -> TunableGroups:
         """
         Get the configuration space of the given environment.
 
@@ -148,7 +171,7 @@ def tunable_params(self):
         """
         return self._tunable_params
 
-    def setup(self, tunables: TunableGroups, global_config: dict = None) -> bool:
+    def setup(self, tunables: TunableGroups, global_config: Optional[dict] = None) -> bool:
         """
         Set up a new benchmark environment, if necessary. This method must be
         idempotent, i.e., calling it several times in a row should be
@@ -170,15 +193,18 @@ def setup(self, tunables: TunableGroups, global_config: dict = None) -> bool:
         _LOG.info("Setup %s :: %s", self, tunables)
         assert isinstance(tunables, TunableGroups)
 
+        if global_config is None:
+            global_config = {}
+
         self._params = self._combine_tunables(tunables)
-        for key in set(self._params).intersection(global_config or {}):
+        for key in set(self._params).intersection(global_config):
             self._params[key] = global_config[key]
         if _LOG.isEnabledFor(logging.DEBUG):
             _LOG.debug("Combined parameters:\n%s", json.dumps(self._params, indent=2))
 
         return True
 
-    def teardown(self):
+    def teardown(self) -> None:
         """
         Tear down the benchmark environment. This method must be idempotent,
         i.e., calling it several times in a row should be equivalent to a

mlos_bench/mlos_bench/environment/composite_env.py

@@ -7,7 +7,7 @@
 """
 
 import logging
-from typing import Optional, Tuple
+from typing import List, Optional, Tuple
 
 from mlos_bench.service.base_service import Service
 from mlos_bench.environment.status import Status
@@ -22,8 +22,8 @@ class CompositeEnv(Environment):
     Composite benchmark environment.
     """
 
-    def __init__(self, name: str, config: dict, global_config: dict = None,
-                 tunables: TunableGroups = None, service: Service = None):
+    def __init__(self, name: str, config: dict, global_config: Optional[dict] = None,
+                 tunables: Optional[TunableGroups] = None, service: Optional[Service] = None):
         # pylint: disable=too-many-arguments
         """
         Create a new environment with a given config.
@@ -46,29 +46,29 @@ def __init__(self, name: str, config: dict, global_config: dict = None,
         """
         super().__init__(name, config, global_config, tunables, service)
 
-        self._children = []
+        self._children: List[Environment] = []
 
         for child_config_file in config.get("include_children", []):
-            for env in self._service.load_environment_list(
+            for env in self._config_loader_service.load_environment_list(
                     child_config_file, global_config, tunables, self._service):
                 self._add_child(env)
 
         for child_config in config.get("children", []):
-            self._add_child(self._service.build_environment(
+            self._add_child(self._config_loader_service.build_environment(
                 child_config, global_config, tunables, self._service))
 
         if not self._children:
             raise ValueError("At least one child environment must be present")
 
-    def _add_child(self, env: Environment):
+    def _add_child(self, env: Environment) -> None:
         """
         Add a new child environment to the composite environment.
         This method is called from the constructor only.
         """
         self._children.append(env)
         self._tunable_params.update(env.tunable_params())
 
-    def setup(self, tunables: TunableGroups, global_config: dict = None) -> bool:
+    def setup(self, tunables: TunableGroups, global_config: Optional[dict] = None) -> bool:
         """
         Set up the children environments.
 
@@ -92,7 +92,7 @@ def setup(self, tunables: TunableGroups, global_config: dict = None) -> bool:
         )
         return self._is_ready
 
-    def teardown(self):
+    def teardown(self) -> None:
         """
         Tear down the children environments. This method is idempotent,
         i.e., calling it several times is equivalent to a single call.