WIP very confused...

The handling of various parameters is VERY confusing. What is necessary for CMR
vs subsetting/ordering is not clear and conifg is mutated by various parts of
the code, making it difficult to track down where values come from/get set/overridden

icepyx/core/APIformatting.py

@@ -265,6 +265,9 @@ def __init__(
         self,
         partype: T,
         values: Optional[dict] = None,
+        # TODO: 'download" reqtype appears to never get used. `None` is most
+        # common, and `search` is passed in when creating required cmr
+        # parameters to search for matching granules.
         reqtype: Optional[Literal["search", "download"]] = None,
     ):
         assert partype in [

icepyx/core/granules.py

@@ -249,6 +249,7 @@ def get_avail(
                 headers=headers,
                 params=apifmt.to_string(params),
             )
+            breakpoint()
 
             try:
                 cmr_search_after = response.headers["CMR-Search-After"]
@@ -287,7 +288,6 @@ def get_avail(
     def place_order(
         self,
         CMRparams: CMRParams,
-        reqparams,  # : EGIRequiredParamsDownload,
         subsetparams,
         verbose,
         subset=True,
@@ -305,7 +305,7 @@ def place_order(
             Dictionary of properly formatted CMR search parameters.
         reqparams :
             Dictionary of properly formatted parameters required for searching, ordering,
-            or downloading from NSIDC (via their EGI system).
+            or downloading from NSASA (via harmony).
         subsetparams : dictionary
             Dictionary of properly formatted subsetting parameters. An empty dictionary
             is passed as input here when subsetting is set to False in query methods.
@@ -336,26 +336,22 @@ def place_order(
 
         # TODO: this returns granules for collections that do not match our
         # query. Why? Is this expected or a bug?
-        self.get_avail(CMRparams, reqparams)
+        breakpoint()
+        self.get_avail(CMRparams)
 
         # TODO: the harmony API may not support non-subsetting. So we may need
         # to provide a list of granules for harmony to download, or use a
         # different API.
         if subset is False:
-            request_params = apifmt.combine_params(
-                CMRparams, reqparams, {"agent": "NO"}
-            )
+            request_params = apifmt.combine_params(CMRparams, {"agent": "NO"})
         else:
-            request_params = apifmt.combine_params(CMRparams, reqparams, subsetparams)
+            request_params = apifmt.combine_params(CMRparams, subsetparams)
 
         concept_id = get_concept_id(
-            product=request_params["short_name"], version=request_params["version"]
+            product=request_params["short_name"],
+            version=request_params["version"],
         )
 
-        # TODO: At this point, the request parameters have been formatted into
-        # strings. `harmony-py` expects python objects (e.g., `dt.datetime` for
-        # temporal values)
-
         # Place the order.
         # TODO: there are probably other options we want to more generically
         # expose here. E.g., instead of just accepting a `bounding_box` of a
@@ -381,6 +377,8 @@ def place_order(
         # "paused", or "canceled" are documented here:
         # https://harmony.earthdata.nasa.gov/docs#getting-job-status
         # I have also seen `running` and `running_with_errors`.
+        # The list of possible statues are here:
+        # https://github.com/nasa/harmony/blob/8b2eb47feab5283d237f3679ac8e09f50e85038f/db/db.sql#L8
         while status["status"].startswith("running"):
             print(
                 "Your harmony order status is still ",

icepyx/core/harmony.py

@@ -59,6 +59,6 @@ def place_order(
 
         return job_id
 
-    def check_order_status(self, job_id: str):
+    def check_order_status(self, job_id: str) -> dict[str, Any]:
         status = self.harmony_client.status(job_id)
         return status

icepyx/core/query.py

@@ -571,8 +571,13 @@ def CMRparams(self) -> CMRParams:
         # print(self._CMRparams)
         # print(self._CMRparams.fmted_keys)
 
-        # dictionary of optional CMR parameters
-        kwargs = {}
+        # create a dictionary of required and optional CMR parameters.
+        # Start by copying the required keys, formatted for CMR requests.
+        # TODO: This gets set at some point in the code, but later `CMRparams`
+        # lacks the values in `cmr_reqparams`! Why??
+        self.cmr_reqparams
+        kwargs = self._cmr_reqparams.fmted_keys.copy()
+
         # temporal CMR parameters
         if hasattr(self, "_temporal") and self.product != "ATL11":
             kwargs["start"] = self._temporal._start
@@ -594,9 +599,9 @@ def CMRparams(self) -> CMRParams:
         return self._CMRparams.fmted_keys
 
     @property
-    def reqparams(self):  # -> EGIRequiredParams:
+    def cmr_reqparams(self):
         """
-        Display the required key:value pairs that will be submitted.
+        Display the required key:value pairs that will be submitted for a CMR request.
         It generates the dictionary if it does not already exist.
 
         Examples
@@ -611,10 +616,12 @@ def reqparams(self):  # -> EGIRequiredParams:
         {'short_name': 'ATL06', 'version': '006', 'page_size': 2000, 'page_num': 1, 'request_mode': 'async', 'include_meta': 'Y', 'client_string': 'icepyx'}
         """
         if not hasattr(self, "_reqparams"):
-            self._reqparams = apifmt.Parameters("required", reqtype="search")
-            self._reqparams.build_params(product=self.product, version=self._version)
+            self._cmr_reqparams = apifmt.Parameters("required", reqtype="search")
+            self._cmr_reqparams.build_params(
+                product=self.product, version=self._version
+            )
 
-        return self._reqparams.fmted_keys
+        return self._cmr_reqparams.fmted_keys
 
     # @property
     # DevQuestion: if I make this a property, I get a "dict" object is not callable
@@ -955,7 +962,7 @@ def avail_granules(
         try:
             self.granules.avail
         except AttributeError:
-            self.granules.get_avail(self.CMRparams, self.reqparams)
+            self.granules.get_avail(self.CMRparams)
 
         if ids or cycles or tracks or cloud:
             # list of outputs in order of ids, cycles, tracks, cloud
@@ -1023,19 +1030,17 @@ def order_granules(
         # breakpoint()
         # raise RefactoringException
 
-        # TODO: this probably shouldn't be mutated based on which method is being called...
-        # It is also very confusing to have both `self.reqparams` and
-        # `self._reqparams`, each of which does something different!
-        self.reqparams
-        if self._reqparams._reqtype == "search":
-            self._reqparams._reqtype = "download"
+        # This call ensures that `self._cmr_reqparams` is set.
+        self.cmr_reqparams
+        if self._cmr_reqparams._reqtype == "search":
+            self._cmr_reqparams._reqtype = "download"
 
-        if "email" in self._reqparams.fmted_keys or email is False:
-            self._reqparams.build_params(**self._reqparams.fmted_keys)
+        if "email" in self._cmr_reqparams.fmted_keys or email is False:
+            self._cmr_reqparams.build_params(**self._cmr_reqparams.fmted_keys)
         elif email is True:
             user_profile = self.auth.get_user_profile()  # pyright: ignore[reportAttributeAccessIssue]
-            self._reqparams.build_params(
-                **self._reqparams.fmted_keys, email=user_profile["email_address"]
+            self._cmr_reqparams.build_params(
+                **self._cmr_reqparams.fmted_keys, email=user_profile["email_address"]
             )
 
         if subset is False:
@@ -1051,19 +1056,26 @@ def order_granules(
         # and also if it already has a list of avail granules (if not, need to create one and add session)
 
         # Place multiple orders, one per granule, if readable_granule_name is used.
+        # TODO/Question: is "readable granule name" a thing in harmony? This may
+        # only be relevant for non-subset requests that will be handled by e.g.,
+        # `earthaccess`.
+        # Answer: there appears to be a `granuleName` parameter that harmony can
+        # take for its own internal queries to CMR, but it is unclear how that
+        # works. See https://harmony.earthdata.nasa.gov/docs#query-parameters
+        # `harmony-py` accepts `granule_name` as an input.
         if "readable_granule_name[]" in self.CMRparams:
             gran_name_list = self.CMRparams["readable_granule_name[]"]
             tempCMRparams = self.CMRparams
             if len(gran_name_list) > 1:
                 print(
-                    "NSIDC only allows ordering of one granule by name at a time; your orders will be placed accordingly."
+                    "Harmony only allows ordering of one granule by name at a time;"
+                    " your orders will be placed accordingly."
                 )
             for gran in gran_name_list:
                 tempCMRparams["readable_granule_name[]"] = gran
                 self.granules.place_order(
                     tempCMRparams,
-                    self.reqparams,
-                    self.subsetparams(**kwargs),
+                    self.subsetparams(granule_name=gran, **kwargs),
                     verbose,
                     subset,
                     geom_filepath=self._spatial._geom_file,
@@ -1072,7 +1084,6 @@ def order_granules(
         else:
             self.granules.place_order(
                 self.CMRparams,
-                self.reqparams,
                 self.subsetparams(**kwargs),
                 verbose,
                 subset,