Refactor/add typing (#74)

* refactor(client): Add static typing Signed-off-by: GustaafL <[email protected]> * refactor(client): Add error checking for wrong content type Signed-off-by: GustaafL <[email protected]> * refactor(client): type client.py Signed-off-by: GustaafL <[email protected]> * refactor(cem): type as much some of the code Signed-off-by: GustaafL <[email protected]> * feat(mypy): add mypy to pre-commit checks Signed-off-by: GustaafL <[email protected]> * refactor(typing): add types for constraints and async parts Signed-off-by: GustaafL <[email protected]> * Update ci.yml Signed-off-by: Guus Linzel <[email protected]> * refactor(tests): remove commented code Signed-off-by: GustaafL <[email protected]> * refactor(errors): only use message in ContentTypeError Signed-off-by: GustaafL <[email protected]> * refactor(errors): raise specific errors when validating client inputs Signed-off-by: GustaafL <[email protected]> * refactor(cem): Logger typing changed to avoid accidental sharing Signed-off-by: GustaafL <[email protected]> * refactor(test): import future annotations Signed-off-by: GustaafL <[email protected]> * Update ci.yml Signed-off-by: Guus Linzel <[email protected]> --------- Signed-off-by: GustaafL <[email protected]> Signed-off-by: Guus Linzel <[email protected]>
FlexMeasures · Feb 16, 2024 · f718fa6 · f718fa6
1 parent 4ca8290
commit f718fa6
Show file tree

Hide file tree

Showing 9 changed files with 207 additions and 85 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -65,3 +65,12 @@ repos:
 #   rev: v2.2.2
 #   hooks:
 #   - id: codespell
+
+- repo: local
+  hooks:
+  - id: mypy
+    name: mypy (static typing)
+    pass_filenames: false
+    language: script
+    entry: ci/run_mypy.sh
+    verbose: true
diff --git a/ci/run_mypy.sh b/ci/run_mypy.sh
@@ -0,0 +1,10 @@
+#!/bin/bash
+set -e
+pip install --upgrade 'mypy>=0.902'
+pip install types-pytz types-requests types-Flask types-click types-redis types-tzlocal types-python-dateutil types-setuptools types-tabulate types-PyYAML
+# We are checking python files which have type hints, and leave out bigger issues we made issues for
+# * data/scripts: We'll remove legacy code: https://trello.com/c/1wEnHOkK/7-remove-custom-data-scripts
+# * data/models and data/services: https://trello.com/c/rGxZ9h2H/540-makequery-call-signature-is-incoherent
+    files=$(find src \
+    -name \*.py | xargs grep -l "from typing import")
+mypy --follow-imports skip --ignore-missing-imports $files
diff --git a/src/flexmeasures_client/client.py b/src/flexmeasures_client/client.py
@@ -42,38 +42,42 @@ class FlexMeasuresClient:
     ssl: bool = False
     api_version: str = API_VERSION
     path: str = f"/api/{api_version}/"
-    access_token: str = None
+    access_token: str | None = None
 
     max_polling_steps: int = MAX_POLLING_STEPS
     polling_timeout: float = POLLING_TIMEOUT  # seconds
     request_timeout: float = REQUEST_TIMEOUT  # seconds
     polling_interval: float = POLLING_INTERVAL  # seconds
-    session: ClientSession | None = None
+    session: ClientSession = ClientSession()
 
     def __post_init__(self):
         if not re.match(r".+\@.+\..+", self.email):
-            raise ValueError(f"{self.email} is not an email address format string")
+            raise EmailValidationError(
+                f"{self.email} is not an email address format string"
+            )
         if self.api_version not in API_VERSIONS_LIST:
-            raise ValueError(f"version not in versions list: {API_VERSIONS_LIST}")
+            raise WrongAPIVersionError(
+                f"Version {self.api_version} not in versions list: {API_VERSIONS_LIST}"
+            )
         # if ssl then scheme is https.
         if self.ssl:
             self.scheme = "https"
         else:
             self.scheme = "http"
         if re.match(r"^http\:\/\/", self.host):
             host_without_scheme = self.host.removeprefix("http://")
-            raise ValueError(
+            raise WrongHostError(
                 f"http:// should not be included in {self.host}."
                 f"Instead use host={host_without_scheme}"
             )
         if re.match(r"^https\:\/\/", self.host):
             host_without_scheme = self.host.removeprefix("https://")
-            raise ValueError(
+            raise WrongHostError(
                 f"https:// should not be included in {self.host}."
                 f"To use https:// set ssl=True and host={host_without_scheme}"
             )
         if len(self.password) < 1:
-            raise ValueError("password cannot be empty")
+            raise EmptyPasswordError("password cannot be empty")
 
     async def close(self):
         """Function to close FlexMeasuresClient session when all requests are done"""
@@ -83,7 +87,7 @@ async def request(
         self,
         uri: str,
         *,
-        json: dict | None = None,
+        json_payload: dict | None = None,
         method: str = "POST",
         path: str = path,
         params: dict[str, Any] | None = None,
@@ -123,7 +127,7 @@ async def request(
                                 url=url,
                                 params=params,
                                 headers=headers,
-                                json=json,
+                                json_payload=json_payload,
                                 polling_step=polling_step,
                                 reauth_once=reauth_once,
                             )
@@ -154,12 +158,12 @@ async def request_once(
         url: URL,
         params: dict[str, Any] | None = None,
         headers: dict | None = None,
-        json: dict | None = None,
+        json_payload: dict | None = None,
         polling_step: int = 0,
         reauth_once: bool = True,
     ) -> tuple[ClientResponse, int, bool, str]:
         url_msg = f"url: {url}"
-        json_msg = f"payload: {json}"
+        json_msg = f"payload: {json_payload}"
         params_msg = f"params: {params}"
         method_msg = f"method: {method}"
         headers_msg = f"headers: {headers}"
@@ -177,7 +181,7 @@ async def request_once(
             url=url,
             params=params,
             headers=headers,
-            json=json,
+            json=json_payload,
             ssl=self.ssl,
             allow_redirects=False,
         )
@@ -223,7 +227,7 @@ async def get_access_token(self):
         response, _status = await self.request(
             uri="requestAuthToken",
             path="/api/",
-            json={
+            json_payload={
                 "email": self.email,
                 "password": self.password,
             },
@@ -244,7 +248,7 @@ async def post_measurements(
         Post sensor data for the given time range.
         This function raises a ValueError when an unhandled status code is returned
         """
-        json = dict(
+        json_payload = dict(
             sensor=f"{ENTITY_ADDRESS_PLACEHOLDER}.{sensor_id}",
             start=pd.Timestamp(
                 start
@@ -254,11 +258,11 @@ async def post_measurements(
             unit=unit,
         )
         if prior:
-            json["prior"] = prior
+            json_payload["prior"] = prior
 
         _response, status = await self.request(
             uri="sensors/data",
-            json=json,
+            json_payload=json_payload,
         )
         check_for_status(status, 200)
         logging.info("Sensor data sent successfully.")
@@ -287,6 +291,10 @@ async def get_schedule(
             },
         )
         check_for_status(status, 200)
+        if not isinstance(schedule, dict):
+            raise ContentTypeError(
+                f"Expected a dictionary schedule, but got {type(schedule)}",
+            )
         return schedule
 
     async def get_assets(self) -> list[dict]:
@@ -298,6 +306,11 @@ async def get_assets(self) -> list[dict]:
         """
         assets, status = await self.request(uri="assets", method="GET")
         check_for_status(status, 200)
+
+        if not isinstance(assets, list):
+            raise ContentTypeError(
+                f"Expected a list of assets, but got {type(assets)}",
+            )
         return assets
 
     async def get_sensors(self) -> list[dict]:
@@ -307,6 +320,10 @@ async def get_sensors(self) -> list[dict]:
         """
         sensors, status = await self.request(uri="sensors", method="GET")
         check_for_status(status, 200)
+        if not isinstance(sensors, list):
+            raise ContentTypeError(
+                f"Expected a list of sensors, but got {type(sensors)}",
+            )
         return sensors
 
     async def trigger_and_get_schedule(
@@ -377,6 +394,10 @@ async def get_sensor_data(
             uri="sensors/data", method="GET", params=params
         )
         check_for_status(status, 200)
+        if not isinstance(response, dict):
+            raise ContentTypeError(
+                f"Expected a sensor data dictionary, but got {type(response)}",
+            )
         data_fields = ("values", "start", "duration", "unit")
         sensor_data = {k: v for k, v in response.items() if k in data_fields}
         return sensor_data
@@ -399,9 +420,13 @@ async def get_sensor(self, sensor_id: int) -> dict:
         This function raises a ValueError when an unhandled status code is returned
         """
         uri = f"sensors/{sensor_id}"
-        response, status = await self.request(uri=uri, method="GET")
+        sensor, status = await self.request(uri=uri, method="GET")
         check_for_status(status, 200)
-        return response
+        if not isinstance(sensor, dict):
+            raise ContentTypeError(
+                f"Expected a sensor dictionary, but got {type(sensor)}",
+            )
+        return sensor
 
     async def add_sensor(
         self,
@@ -439,9 +464,15 @@ async def add_sensor(
         if attributes:
             sensor["attributes"] = json.dumps(attributes)
         uri = "sensors"
-        response, status = await self.request(uri=uri, json=sensor, method="POST")
+        new_sensor, status = await self.request(
+            uri=uri, json_payload=sensor, method="POST"
+        )
         check_for_status(status, 201)
-        return response
+        if not isinstance(new_sensor, dict):
+            raise ContentTypeError(
+                f"Expected a sensor dictionary, but got {type(new_sensor)}",
+            )
+        return new_sensor
 
     async def add_asset(
         self,
@@ -479,9 +510,15 @@ async def add_asset(
             asset["attributes"] = json.dumps(attributes)
 
         uri = "assets"
-        response, status = await self.request(uri=uri, json=asset, method="POST")
+        new_asset, status = await self.request(
+            uri=uri, json_payload=asset, method="POST"
+        )
         check_for_status(status, 201)
-        return response
+        if not isinstance(new_asset, dict):
+            raise ContentTypeError(
+                f"Expected an asset dictionary, but got {type(new_asset)}",
+            )
+        return new_asset
 
     async def update_asset(self, asset_id: int, updates: dict) -> dict:
         """Patch an asset
@@ -503,9 +540,15 @@ async def update_asset(self, asset_id: int, updates: dict) -> dict:
         uri = f"assets/{asset_id}"
         if updates.get("attributes"):
             updates["attributes"] = json.dumps(updates["attributes"])
-        response, status = await self.request(uri=uri, json=updates, method="PATCH")
+        updated_asset, status = await self.request(
+            uri=uri, json_payload=updates, method="PATCH"
+        )
         check_for_status(status, 200)
-        return response
+        if not isinstance(updated_asset, dict):
+            raise ContentTypeError(
+                f"Expected an asset dictionary, but got {type(updated_asset)}",
+            )
+        return updated_asset
 
     async def update_sensor(self, sensor_id: int, updates: dict) -> dict:
         """Patch a sensor
@@ -527,10 +570,16 @@ async def update_sensor(self, sensor_id: int, updates: dict) -> dict:
         uri = f"sensors/{sensor_id}"
         if updates.get("attributes"):
             updates["attributes"] = json.dumps(updates["attributes"])
-        response, status = await self.request(uri=uri, json=updates, method="PATCH")
+        updated_sensor, status = await self.request(
+            uri=uri, json_payload=updates, method="PATCH"
+        )
         # Raise ValueError
         check_for_status(status, 200)
-        return response
+        if not isinstance(updated_sensor, dict):
+            raise ContentTypeError(
+                f"Expected a sensor dictionary, but got {type(updated_sensor)}",
+            )
+        return updated_sensor
 
     async def trigger_schedule(
         self,
@@ -539,7 +588,7 @@ async def trigger_schedule(
         duration: str | timedelta,
         flex_model: dict,
         flex_context: dict,
-    ):
+    ) -> str:
         message = {
             "start": pd.Timestamp(
                 start
@@ -550,12 +599,21 @@ async def trigger_schedule(
         }
         response, status = await self.request(
             uri=f"sensors/{sensor_id}/schedules/trigger",
-            json=message,
+            json_payload=message,
         )
         check_for_status(status, 200)
+
         logging.info("Schedule triggered successfully.")
+        if not isinstance(response, dict):
+            raise ContentTypeError(
+                f"Expected a dictionary, but got {type(response)}",
+            )
 
-        schedule_id: str = response.get("schedule")
+        if not isinstance(response.get("schedule"), str):
+            raise ContentTypeError(
+                f"Expected a schedule ID, but got {type(response.get('schedule'))}",
+            )
+        schedule_id = response["schedule"]
         return schedule_id
 
     @staticmethod
@@ -569,7 +627,7 @@ def create_storage_flex_model(
         storage_efficiency: float | None = None,
         soc_minima: list | None = None,
         soc_maxima: list | None = None,
-    ):
+    ) -> dict:
         flex_model = {
             "soc-unit": soc_unit,
             "soc-at-start": soc_at_start,
@@ -592,13 +650,14 @@ def create_storage_flex_model(
 
         return flex_model
 
+    # add type hints
     @staticmethod
     def create_storage_flex_context(
         consumption_price_sensor: int | None = None,
         production_price_sensor: int | None = None,
-        inflexible_device_sensors: list[int] | None = None,
-    ):
-        flex_context = {}
+        inflexible_device_sensors: int | list[int] | None = None,
+    ) -> dict:
+        flex_context: dict = {}
         # Set optional flex context
         if consumption_price_sensor is not None:
             flex_context["consumption-price-sensor"] = consumption_price_sensor
@@ -610,7 +669,7 @@ def create_storage_flex_context(
         return flex_context
 
     @staticmethod
-    def convert_units(values: list[int | float], from_unit: str, to_unit: str) -> dict:
+    def convert_units(values: list[int | float], from_unit: str, to_unit: str) -> list:
         """Convert values between W, kW and MW, as required."""
         if from_unit == "MW" and to_unit == "W":
             values = [v * 10**6 for v in values]
@@ -631,3 +690,43 @@ def convert_units(values: list[int | float], from_unit: str, to_unit: str) -> di
                 f"Power conversion from {from_unit} to {to_unit} is not supported."
             )
         return values
+
+
+class ContentTypeError(Exception):
+    """Raised when the response from the API is not in the expected format"""
+
+    def __init__(self, message):
+        self.message = message
+        super().__init__(self.message)
+
+
+class WrongHostError(Exception):
+    """Raised when the host includes the scheme (http:// or https://)"""
+
+    def __init__(self, message):
+        self.message = message
+        super().__init__(self.message)
+
+
+class EmptyPasswordError(Exception):
+    """Raised when the password is empty"""
+
+    def __init__(self, message):
+        self.message = message
+        super().__init__(self.message)
+
+
+class EmailValidationError(Exception):
+    """Raised when the email is not in the correct format"""
+
+    def __init__(self, message):
+        self.message = message
+        super().__init__(self.message)
+
+
+class WrongAPIVersionError(Exception):
+    """Raised when the API version is not in the versions list"""
+
+    def __init__(self, message):
+        self.message = message
+        super().__init__(self.message)