feat: response to mr comments

Author: Hugh Cutcher

.gitignore

@@ -2,4 +2,5 @@ public
 .idea
 *.ipynb_checkpoints
 .pytest_cache
-tests/__pycache__
+tests/__pycache__
+*.pyc

solcast/api.py

@@ -1,10 +1,11 @@
+import copy
 import json
 import os
 from dataclasses import dataclass
 from urllib.request import urlopen, Request
 import urllib.parse
 import urllib.error
-from typing import Dict, Optional
+from typing import Optional
 
 import solcast
 
@@ -24,10 +25,11 @@ class Response:
     url: str
     data: Optional[bytes]
     success: bool
+    method: str
     exception: Optional[str] = None
 
     def __repr__(self):
-        return f"status code={self.code}, url={self.url}"
+        return f"status code={self.code}, url={self.url}, method={self.method}"
 
     def to_dict(self):
         if self.code != 200:
@@ -78,22 +80,26 @@ def __init__(self, base_url: str, endpoint: str):
         self.url = self.make_url()
 
     @staticmethod
-    def check_params(params: dict) -> (dict, str):
-        """runs some basic checks on the parameters that will be passed in the
-        GET request."""
+    def _check_params(params: dict) -> (dict, str):
+        """Run basic checks on the parameters that will be passed to the HTTP request."""
         assert isinstance(params, dict), "parameters needs to be a dict"
+        params = copy.deepcopy(params)
 
-        if "api_key" not in params:
-            params.update({"api_key": os.getenv("SOLCAST_API_KEY")})
+        if "api_key" in params:
+            # api key in the header for secrecy
+            key = params["api_key"]
+            del params["api_key"]
+        else:
+            key = os.getenv("SOLCAST_API_KEY")
 
-        if params["api_key"] is None:
+        if key is None:
             raise ValueError(
                 "no API key provided. Either set it as an environment "
                 "variable SOLCAST_API_KEY, or provide `api_key` "
                 "as an argument. Visit https://solcast.com to get an API key."
             )
 
-        if len(params["api_key"]) <= 1:
+        if len(key) <= 1:
             raise ValueError("API key is too short.")
 
         if "output_parameters" in params.keys() and isinstance(
@@ -109,18 +115,15 @@ def check_params(params: dict) -> (dict, str):
 
         # only json supported
         if "format" in params.keys():
-            assert (
-                params["format"] == "json"
-            ), "only json response format is currently supported."
-
-        # api key in the header for secrecy
-        key = params["api_key"]
-        del params["api_key"]
+            if params["format"] != "json":
+                raise NotImplementedError(
+                    "Only json response format is currently supported."
+                )
 
         return params, key
 
     def make_url(self) -> str:
-        """composes the full URL."""
+        """Compose the full URL."""
         return "/".join([self.base_url, self.endpoint])
 
     def get(self, params: dict) -> Response:
@@ -183,7 +186,7 @@ def delete(self, params: dict) -> Response:
         """
         return self._make_request(params, method="DELETE")
 
-    def _make_request(self, params: Dict, method: str) -> Response:
+    def _make_request(self, params: dict, method: str) -> Response:
         """Make a request using urllib with the HTTP method specified
 
         Args:
@@ -194,7 +197,7 @@ def _make_request(self, params: Dict, method: str) -> Response:
             a Response object.
         """
 
-        params, key = self.check_params(params)
+        params, key = self._check_params(params)
         url = self.url + "?" + urllib.parse.urlencode(params)
         req = Request(
             url,
@@ -205,7 +208,12 @@ def _make_request(self, params: Dict, method: str) -> Response:
             with urlopen(req) as response:
                 body = response.read()
                 return Response(
-                    code=response.code, url=url, data=body, success=True, exception=None
+                    code=response.code,
+                    url=url,
+                    data=body,
+                    success=True,
+                    exception=None,
+                    method=method,
                 )
         except urllib.error.HTTPError as e:
             try:
@@ -218,4 +226,5 @@ def _make_request(self, params: Dict, method: str) -> Response:
                 data=None,
                 exception=exception_message,
                 success=False,
+                method=method,
             )

solcast/forecast.py

@@ -22,6 +22,7 @@ def radiation_and_weather(
         latitude: in decimal degrees, between -90 and 90, north is positive
         longitude: in decimal degrees, between -180 and 180, east is positive
         output_parameters: list of strings with the parameters to return
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
     """
@@ -51,6 +52,7 @@ def rooftop_pv_power(
         latitude: in decimal degrees, between -90 and 90, north is positive
         longitude: in decimal degrees, between -180 and 180, east is positive
         output_parameters: list of strings with the parameters to return
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
     """
@@ -76,6 +78,7 @@ def advanced_pv_power(resource_id: int, **kwargs) -> Response:
 
     Args:
         resource_id: a Solcast resource id
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
     """

solcast/historic.py

@@ -23,6 +23,7 @@ def radiation_and_weather(
         end: optional, datetime-like, last day of the requested period
         duration: optional, ISO_8601 compliant duration for the historic data.
             Must be within 31 days of the start_date.
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
     """
@@ -65,6 +66,7 @@ def rooftop_pv_power(
         end: optional, datetime-like, last day of the requested period
         duration: optional, ISO_8601 compliant duration for the historic data.
             Must be within 31 days of the start_date.
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
     """

solcast/live.py

@@ -20,6 +20,7 @@ def radiation_and_weather(
         latitude: in decimal degrees, between -90 and 90, north is positive
         longitude: in decimal degrees, between -180 and 180, east is positive
         output_parameters: list of strings with the parameters to return
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
     """
@@ -47,6 +48,7 @@ def rooftop_pv_power(latitude: float, longitude: float, **kwargs) -> Response:
     Args:
         latitude: in decimal degrees, between -90 and 90, north is positive
         longitude: in decimal degrees, between -180 and 180, east is positive
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
     """
@@ -66,6 +68,7 @@ def advanced_pv_power(resource_id: int, **kwargs) -> Response:
 
     Args:
         resource_id: a Solcast resource id
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
     """

solcast/pv_power_sites.py

@@ -2,8 +2,15 @@
 from .urls import base_url, pv_power_site, pv_power_sites
 
 
-def list_pv_power_site(**kwargs) -> Response:
-    """List available PV power sites"""
+def list_pv_power_sites(**kwargs) -> Response:
+    """
+    List available PV power sites.
+
+    Args:
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
+
+    See https://docs.solcast.com.au/ for full list of parameters.
+    """
     client = Client(base_url=base_url, endpoint=pv_power_sites)
 
     params = {"format": "json", **kwargs}
@@ -15,11 +22,13 @@ def list_pv_power_site(**kwargs) -> Response:
 
 def get_pv_power_site(resource_id: str, **kwargs) -> Response:
     """
-    Get an existing PV power site's specifications
+    Get an existing PV power site's specifications.
 
     Args:
         resource_id: unique string to identify the location that was generated when creating a site
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
+    See https://docs.solcast.com.au/ for full list of parameters.
     """
     client = Client(base_url=base_url, endpoint=pv_power_site)
 
@@ -37,15 +46,15 @@ def create_pv_power_site(
     **kwargs,
 ) -> Response:
     """
-    Create PV power site to be used with Solcast's advanced PV power model
+    Create PV power site to be used with Solcast's advanced PV power model.
 
     Args:
         name: arbitrary string to identify the location, unique not required
         latitude: in decimal degrees, between -90 and 90, north is positive
         longitude: in decimal degrees, between -180 and 180, east is positive
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for more information and full list of parameters.
-
     """
     client = Client(base_url=base_url, endpoint=pv_power_site)
 
@@ -64,13 +73,13 @@ def create_pv_power_site(
 
 def patch_pv_power_site(resource_id: str, **kwargs) -> Response:
     """
-    Patch an existing PV power site to partially update the site's specifications
+    Patch an existing PV power site to partially update the site's specifications.
 
     Args:
         resource_id: unique string to identify the location that is generated when creating a site
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
-
     """
     client = Client(base_url=base_url, endpoint=pv_power_site)
 
@@ -87,13 +96,13 @@ def patch_pv_power_site(resource_id: str, **kwargs) -> Response:
 
 def update_pv_power_site(resource_id: str, **kwargs) -> Response:
     """
-    Update an existing PV power site's specifications
+    Overwrite an existing PV power site's specifications.
 
     Args:
         resource_id: unique string to identify the location that is generated when creating a site
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
-
     """
     client = Client(base_url=base_url, endpoint=pv_power_site)
 
@@ -110,11 +119,13 @@ def update_pv_power_site(resource_id: str, **kwargs) -> Response:
 
 def delete_pv_power_site(resource_id: str, **kwargs) -> Response:
     """
-    Delete an existing PV power site
+    Delete an existing PV power site.
 
     Args:
         resource_id: unique string to identify the location that is generated when creating a site
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
+    See https://docs.solcast.com.au/ for full list of parameters.
     """
     client = Client(base_url=base_url, endpoint=pv_power_site)
 

solcast/tmy.py

@@ -11,6 +11,7 @@ def radiation_and_weather(latitude: float, longitude: float, **kwargs) -> Respon
     Args:
         latitude: in decimal degrees, between -90 and 90, north is positive
         longitude: in decimal degrees, between -180 and 180, east is positive
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
     """
@@ -31,6 +32,7 @@ def rooftop_pv_power(latitude: float, longitude: float, **kwargs) -> Response:
     Args:
         latitude: in decimal degrees, between -90 and 90, north is positive
         longitude: in decimal degrees, between -180 and 180, east is positive
+        **kwargs: additional keyword arguments to be passed through as URL parameters to the Solcast API
 
     See https://docs.solcast.com.au/ for full list of parameters.
     """

tests/test_client.py

@@ -15,12 +15,12 @@ def mock_short_key(monkeypatch):
 def test_fail_short_key(mock_short_key):
     with pytest.raises(ValueError, match="API key is too short."):
         cls = Client(base_url=base_url, endpoint=live_radiation_and_weather)
-        cls.check_params({"a": None})
+        cls._check_params({"a": None})
 
 
 def test_pass_key_in_params(mock_short_key):
     cls = Client(base_url=base_url, endpoint=live_radiation_and_weather)
-    assert cls.check_params({"api_key": "dummy"})[1] == "dummy"
+    assert cls._check_params({"api_key": "dummy"})[1] == "dummy"
 
 
 def test_client():
@@ -40,7 +40,7 @@ def test_client():
     assert res.code == 200
     assert len(res.to_dict()) == 1
 
-    params, _ = cli.check_params(
+    params, _ = cli._check_params(
         {
             "latitude": -33.8567848776324,
             "longitude": 151.215297,
@@ -55,7 +55,9 @@ def test_client():
 
 def test_response():
     raw_data = b'{"estimated_actuals":[{"ghi":54,"period_end":"2023-06-22T05:30:00.0000000Z","period":"PT30M"}]}'
-    rsp = Response(data=raw_data, url="some_url", code=200, success=True)
+    rsp = Response(
+        data=raw_data, url="some_url", code=200, success=True, method="arbitrary_method"
+    )
 
     assert rsp.success is True
     assert rsp.to_pandas().shape[0] == 1

tests/test_pv_power_sites.py

@@ -2,7 +2,7 @@
 
 
 def test_list_pv_power_sites():
-    res = pv_power_sites.list_pv_power_site()
+    res = pv_power_sites.list_pv_power_sites()
 
     assert res.success
     assert isinstance(res.to_dict(), list)