Partially typecheck APIformatting module (#598)

Co-authored-by: GitHub Action <[email protected]>

icepyx/core/APIformatting.py

@@ -1,8 +1,9 @@
 """Generate and format information for submitting to API (CMR and NSIDC)."""
 
 import datetime as dt
-from typing import Any, Generic, Literal, TypeVar, Union, overload
+from typing import Any, Generic, Literal, Optional, TypeVar, Union, overload
 
+from icepyx.core.exceptions import ExhaustiveTypeGuardException, TypeGuardException
 from icepyx.core.types import (
     CMRParams,
     EGIParamsSubset,
@@ -36,10 +37,6 @@ def _fmt_temporal(start, end, key):
 
     assert isinstance(start, dt.datetime)
     assert isinstance(end, dt.datetime)
-    assert key in [
-        "time",
-        "temporal",
-    ], "An invalid time key was submitted for formatting."
 
     if key == "temporal":
         fmt_timerange = (
@@ -53,6 +50,8 @@ def _fmt_temporal(start, end, key):
             + ","
             + end.strftime("%Y-%m-%dT%H:%M:%S")
         )
+    else:
+        raise ValueError("An invalid time key was submitted for formatting.")
 
     return {key: fmt_timerange}
 
@@ -231,7 +230,7 @@ def __get__(
         Returns the dictionary of formatted keys associated with the
         parameter object.
         """
-        return instance._fmted_keys
+        return instance._fmted_keys  # pyright: ignore[reportReturnType]
 
 
 # ----------------------------------------------------------------------
@@ -257,13 +256,16 @@ class Parameters(Generic[T]):
         on the type of query. Must be one of ['search','download']
     """
 
+    partype: T
+    _reqtype: Optional[Literal["search", "download"]]
     fmted_keys = _FmtedKeysDescriptor()
+    # _fmted_keys: Union[CMRParams, EGISpecificRequiredParams, EGIParamsSubset]
 
     def __init__(
         self,
         partype: T,
-        values=None,
-        reqtype=None,
+        values: Optional[dict] = None,
+        reqtype: Optional[Literal["search", "download"]] = None,
     ):
         assert partype in [
             "CMR",
@@ -282,31 +284,14 @@ def __init__(
         self._fmted_keys = values if values is not None else {}
 
     @property
-    def poss_keys(self):
+    def poss_keys(self) -> dict[str, list[str]]:
         """
         Returns a list of possible input keys for the given parameter object.
         Possible input keys depend on the parameter type (partype).
         """
 
-        if not hasattr(self, "_poss_keys"):
-            self._get_possible_keys()
-
-        return self._poss_keys
-
-    # @property
-    # def wanted_keys(self):
-    #     if not hasattr(_wanted):
-    #         self._wanted = []
-
-    #     return self._wanted
-
-    def _get_possible_keys(self) -> dict[str, list[str]]:
-        """
-        Use the parameter type to get a list of possible parameter keys.
-        """
-
         if self.partype == "CMR":
-            self._poss_keys = {
+            return {
                 "spatial": ["bounding_box", "polygon"],
                 "optional": [
                     "temporal",
@@ -316,7 +301,7 @@ def _get_possible_keys(self) -> dict[str, list[str]]:
                 ],
             }
         elif self.partype == "required":
-            self._poss_keys = {
+            return {
                 "search": ["short_name", "version", "page_size"],
                 "download": [
                     "short_name",
@@ -331,7 +316,7 @@ def _get_possible_keys(self) -> dict[str, list[str]]:
                 ],
             }
         elif self.partype == "subset":
-            self._poss_keys = {
+            return {
                 "spatial": ["bbox", "Boundingshape"],
                 "optional": [
                     "time",
@@ -341,8 +326,17 @@ def _get_possible_keys(self) -> dict[str, list[str]]:
                     "Coverage",
                 ],
             }
+        else:
+            raise ExhaustiveTypeGuardException
+
+    # @property
+    # def wanted_keys(self):
+    #     if not hasattr(_wanted):
+    #         self._wanted = []
 
-    def _check_valid_keys(self):
+    #     return self._wanted
+
+    def _check_valid_keys(self) -> None:
         """
         Checks that any keys passed in with values are valid keys.
         """
@@ -352,13 +346,13 @@ def _check_valid_keys(self):
 
         val_list = list({val for lis in self.poss_keys.values() for val in lis})
 
-        for key in self.fmted_keys:
+        for key in self.fmted_keys:  # pyright: ignore[reportAttributeAccessIssue]
             assert key in val_list, (
                 "An invalid key (" + key + ") was passed. Please remove it using `del`"
             )
 
     # DevNote: can check_req_values and check_values be combined?
-    def check_req_values(self):
+    def check_req_values(self) -> bool:
         """
         Check that all of the required keys have values, if the key was passed in with
         the values parameter.
@@ -367,17 +361,22 @@ def check_req_values(self):
         assert (
             self.partype == "required"
         ), "You cannot call this function for your parameter type"
+
+        if not self._reqtype:
+            raise TypeGuardException
+
         reqkeys = self.poss_keys[self._reqtype]
 
-        if all(keys in self.fmted_keys for keys in reqkeys):
+        if all(keys in self.fmted_keys for keys in reqkeys):  # pyright: ignore[reportAttributeAccessIssue]
             assert all(
-                self.fmted_keys.get(key, -9999) != -9999 for key in reqkeys
+                self.fmted_keys.get(key, -9999) != -9999  # pyright: ignore[reportAttributeAccessIssue]
+                for key in reqkeys
             ), "One of your formatted parameters is missing a value"
             return True
         else:
             return False
 
-    def check_values(self):
+    def check_values(self) -> bool:
         """
         Check that the non-required keys have values, if the key was
         passed in with the values parameter.
@@ -391,7 +390,8 @@ def check_values(self):
         # not the most robust check, but better than nothing...
         if any(keys in self._fmted_keys for keys in spatial_keys):
             assert any(
-                self.fmted_keys.get(key, -9999) != -9999 for key in spatial_keys
+                self.fmted_keys.get(key, -9999) != -9999  # pyright: ignore[reportAttributeAccessIssue]
+                for key in spatial_keys
             ), "One of your formatted parameters is missing a value"
             return True
         else:
@@ -427,6 +427,9 @@ def build_params(self, **kwargs) -> None:
             self._check_valid_keys()
 
         if self.partype == "required":
+            if not self._reqtype:
+                raise TypeGuardException
+
             if self.check_req_values() and kwargs == {}:
                 pass
             else:
@@ -484,6 +487,7 @@ def build_params(self, **kwargs) -> None:
                 if any(keys in self._fmted_keys for keys in spatial_keys):
                     pass
                 else:
+                    k = None
                     if self.partype == "CMR":
                         k = kwargs["extent_type"]
                     elif self.partype == "subset":
@@ -492,6 +496,9 @@ def build_params(self, **kwargs) -> None:
                         elif kwargs["extent_type"] == "polygon":
                             k = "Boundingshape"
 
+                    if not k:
+                        raise TypeGuardException
+
                     self._fmted_keys.update({k: kwargs["spatial_extent"]})
 
 

icepyx/core/exceptions.py

@@ -1,3 +1,10 @@
+ISSUE_REPORTING_INSTRUCTIONS = (
+    "If you are a user seeing this message, the developers of this software have made a"
+    " mistake! Please report the full error traceback in the icepyx GitHub repository:"
+    " <https://github.com/icesat2py/icepyx/issues/new>"
+)
+
+
 class DeprecationError(Exception):
     """
     Class raised for use of functionality that is no longer supported by icepyx.
@@ -24,3 +31,25 @@ def __init__(
 
     def __str__(self):
         return f"{self.msgtxt}: {self.errmsg}"
+
+
+class TypeGuardException(Exception):
+    """
+    Should never be raised at runtime.
+
+    Used in cases where a runtime check is not desired, but we want to add a "type guard"
+    (https://github.com/microsoft/pyright/blob/main/docs/type-concepts-advanced.md#type-guards)
+    to give the type checker more information.
+    """
+
+    def __str__(self):
+        return ISSUE_REPORTING_INSTRUCTIONS
+
+
+class ExhaustiveTypeGuardException(TypeGuardException):
+    """
+    Should never be raised at runtime.
+
+    Used exclusively in cases where the typechecker needs a typeguard to tell it that a
+    check is exhaustive.
+    """

pyproject.toml

@@ -140,7 +140,6 @@ exclude = [
 # DevGoal: Remove all ignores
 ignore = [
   "icepyx/quest/*",
-  "icepyx/core/APIformatting.py",
   "icepyx/core/auth.py",
   "icepyx/core/is2ref.py",
   "icepyx/core/read.py",