Clean up APIformatting module and fix ATL11 temporal kwarg submission (…

…#515) * add atl11 exception for cmr and required temporal parameters * remove unused "default" key in _get_possible_keys * move "short_name" and "version" keys from the CMR list to the required list * utilize EarthdataAuthMixin for Granules (was still explicitly passing session) * Touched files were also cleaned up for linting. Co-authored-by: Whyjay Zheng <[email protected]> Co-authored-by: GitHub Action <[email protected]>
icesat2py · Apr 10, 2024 · ed6a9fd · ed6a9fd
1 parent e5fd7d1
commit ed6a9fd
Show file tree

Hide file tree

Showing 9 changed files with 676 additions and 663 deletions.
diff --git a/.coveragerc b/.coveragerc
@@ -3,5 +3,4 @@ branch = True
 source = icepyx
 omit =
     setup.py
-    test/*
     doc/*
diff --git a/doc/source/user_guide/documentation/classes_dev_uml.svg b/doc/source/user_guide/documentation/classes_dev_uml.svg
diff --git a/doc/source/user_guide/documentation/classes_user_uml.svg b/doc/source/user_guide/documentation/classes_user_uml.svg
diff --git a/doc/source/user_guide/documentation/packages_user_uml.svg b/doc/source/user_guide/documentation/packages_user_uml.svg
diff --git a/icepyx/core/APIformatting.py b/icepyx/core/APIformatting.py
@@ -1,7 +1,6 @@
 # Generate and format information for submitting to API (CMR and NSIDC)
 
 import datetime as dt
-import pprint
 
 
 # ----------------------------------------------------------------------
@@ -134,13 +133,13 @@ def combine_params(*param_dicts):
 
     Examples
     --------
-    >>> CMRparams = {'short_name': 'ATL06', 'version': '002', 'temporal': '2019-02-20T00:00:00Z,2019-02-28T23:59:59Z', 'bounding_box': '-55,68,-48,71'}
-    >>> reqparams = {'page_size': 2000, 'page_num': 1}
+    >>> CMRparams = {'temporal': '2019-02-20T00:00:00Z,2019-02-28T23:59:59Z', 'bounding_box': '-55,68,-48,71'}
+    >>> reqparams = {'short_name': 'ATL06', 'version': '002', 'page_size': 2000, 'page_num': 1}
     >>> ipx.core.APIformatting.combine_params(CMRparams, reqparams)
-    {'short_name': 'ATL06',
-    'version': '002',
-    'temporal': '2019-02-20T00:00:00Z,2019-02-28T23:59:59Z',
+    {'temporal': '2019-02-20T00:00:00Z,2019-02-28T23:59:59Z',
     'bounding_box': '-55,68,-48,71',
+    'short_name': 'ATL06',
+    'version': '002',
     'page_size': 2000,
     'page_num': 1}
     """
@@ -164,17 +163,18 @@ def to_string(params):
 
     Examples
     --------
-    >>> CMRparams = {'short_name': 'ATL06', 'version': '002', 'temporal': '2019-02-20T00:00:00Z,2019-02-28T23:59:59Z', 'bounding_box': '-55,68,-48,71'}
-    >>> reqparams = {'page_size': 2000, 'page_num': 1}
+    >>> CMRparams = {'temporal': '2019-02-20T00:00:00Z,2019-02-28T23:59:59Z',
+    ...             'bounding_box': '-55,68,-48,71'}
+    >>> reqparams = {'short_name': 'ATL06', 'version': '002', 'page_size': 2000, 'page_num': 1}
     >>> params = ipx.core.APIformatting.combine_params(CMRparams, reqparams)
     >>> ipx.core.APIformatting.to_string(params)
-    'short_name=ATL06&version=002&temporal=2019-02-20T00:00:00Z,2019-02-28T23:59:59Z&bounding_box=-55,68,-48,71&page_size=2000&page_num=1'
+    'temporal=2019-02-20T00:00:00Z,2019-02-28T23:59:59Z&bounding_box=-55,68,-48,71&short_name=ATL06&version=002&page_size=2000&page_num=1'
     """
     param_list = []
     for k, v in params.items():
         if isinstance(v, list):
-            for l in v:
-                param_list.append(k + "=" + l)
+            for i in v:
+                param_list.append(k + "=" + i)
         else:
             param_list.append(k + "=" + str(v))
     # return the parameter string
@@ -255,7 +255,6 @@ def _get_possible_keys(self):
 
         if self.partype == "CMR":
             self._poss_keys = {
-                "default": ["short_name", "version"],
                 "spatial": ["bounding_box", "polygon"],
                 "optional": [
                     "temporal",
@@ -266,8 +265,10 @@ def _get_possible_keys(self):
             }
         elif self.partype == "required":
             self._poss_keys = {
-                "search": ["page_size"],
+                "search": ["short_name", "version", "page_size"],
                 "download": [
+                    "short_name",
+                    "version",
                     "page_size",
                     "page_num",
                     "request_mode",
@@ -279,7 +280,6 @@ def _get_possible_keys(self):
             }
         elif self.partype == "subset":
             self._poss_keys = {
-                "default": [],
                 "spatial": ["bbox", "Boundingshape"],
                 "optional": [
                     "time",
@@ -305,6 +305,7 @@ def _check_valid_keys(self):
                 "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):
         """
         Check that all of the required keys have values, if the key was passed in with
@@ -333,22 +334,14 @@ def check_values(self):
             self.partype != "required"
         ), "You cannot call this function for your parameter type"
 
-        default_keys = self.poss_keys["default"]
         spatial_keys = self.poss_keys["spatial"]
 
-        if all(keys in self._fmted_keys.keys() for keys in default_keys):
-            assert all(
-                self.fmted_keys.get(key, -9999) != -9999 for key in default_keys
+        # not the most robust check, but better than nothing...
+        if any(keys in self._fmted_keys.keys() for keys in spatial_keys):
+            assert any(
+                self.fmted_keys.get(key, -9999) != -9999 for key in spatial_keys
             ), "One of your formated parameters is missing a value"
-
-            # not the most robust check, but better than nothing...
-            if any(keys in self._fmted_keys.keys() for keys in spatial_keys):
-                assert any(
-                    self.fmted_keys.get(key, -9999) != -9999 for key in default_keys
-                ), "One of your formated parameters is missing a value"
-                return True
-            else:
-                return False
+            return True
         else:
             return False
 
@@ -360,14 +353,19 @@ def build_params(self, **kwargs):
         Parameters
         ----------
         **kwargs
-            Keyword inputs containing the needed information to build the parameter list, depending on
-            parameter type, if the already formatted key:value is not submitted as a kwarg.
-            May include optional keyword arguments to be passed to the subsetter. Valid keywords
-            are time, bbox OR Boundingshape, format, projection, projection_parameters, and Coverage.
-
-            Keyword argument inputs for 'CMR' may include: dataset (data product), version, start, end, extent_type, spatial_extent
-            Keyword argument inputs for 'required' may include: page_size, page_num, request_mode, include_meta, client_string
-            Keyword argument inputs for 'subset' may include: geom_filepath, start, end, extent_type, spatial_extent
+            Keyword inputs containing the needed information to build the parameter list, depending
+            on parameter type, if the already formatted key:value is not submitted as a kwarg.
+            May include optional keyword arguments to be passed to the subsetter.
+            Valid keywords are time, bbox OR Boundingshape, format, projection,
+            projection_parameters, and Coverage.
+
+            Keyword argument inputs for 'CMR' may include:
+            start, end, extent_type, spatial_extent
+            Keyword argument inputs for 'required' may include:
+            product or short_name, version, page_size, page_num,
+            request_mode, include_meta, client_string
+            Keyword argument inputs for 'subset' may include:
+            geom_filepath, start, end, extent_type, spatial_extent
 
         """
 
@@ -388,31 +386,29 @@ def build_params(self, **kwargs):
                     "include_meta": "Y",
                     "client_string": "icepyx",
                 }
+
                 for key in reqkeys:
-                    if key in kwargs:
+                    if key == "short_name":
+                        try:
+                            self._fmted_keys.update({key: kwargs[key]})
+                        except KeyError:
+                            self._fmted_keys.update({key: kwargs["product"]})
+                    elif key == "version":
+                        self._fmted_keys.update({key: kwargs["version"]})
+                    elif key in kwargs:
                         self._fmted_keys.update({key: kwargs[key]})
                     elif key in defaults:
                         self._fmted_keys.update({key: defaults[key]})
                     else:
                         pass
 
         else:
-            if self.check_values == True and kwargs == None:
+            if self.check_values is True and kwargs is None:
                 pass
             else:
-                default_keys = self.poss_keys["default"]
                 spatial_keys = self.poss_keys["spatial"]
                 opt_keys = self.poss_keys["optional"]
 
-                for key in default_keys:
-                    if key in self._fmted_keys.values():
-                        assert self._fmted_keys[key]
-                    else:
-                        if key == "short_name":
-                            self._fmted_keys.update({key: kwargs["product"]})
-                        elif key == "version":
-                            self._fmted_keys.update({key: kwargs["version"]})
-
                 for key in opt_keys:
                     if key == "Coverage" and key in kwargs.keys():
                         # DevGoal: make there be an option along the lines of Coverage=default, which will get the default variables for that product without the user having to input is2obj.build_wanted_wanted_var_list as their input value for using the Coverage kwarg

diff --git a/icepyx/core/granules.py b/icepyx/core/granules.py
@@ -11,6 +11,7 @@
 import zipfile
 
 import icepyx.core.APIformatting as apifmt
+from icepyx.core.auth import EarthdataAuthMixin
 import icepyx.core.exceptions
 
 
@@ -139,7 +140,7 @@ def gran_IDs(grans, ids=False, cycles=False, tracks=False, dates=False, cloud=Fa
 # DevGoal: this will be a great way/place to manage data from the local file system
 # where the user already has downloaded data!
 # DevNote: currently this class is not tested
-class Granules:
+class Granules(EarthdataAuthMixin):
     """
     Interact with ICESat-2 data granules. This includes finding,
     ordering, and downloading them as well as (not yet implemented) getting already
@@ -157,7 +158,9 @@ def __init__(
         # files=[],
         # session=None
     ):
-        pass
+        # initialize authentication properties
+        EarthdataAuthMixin.__init__(self)
+
         # self.avail = avail
         # self.orderIDs = orderIDs
         # self.files = files
@@ -207,7 +210,7 @@ def get_avail(self, CMRparams, reqparams, cloud=False):
 
         params = apifmt.combine_params(
             CMRparams,
-            {k: reqparams[k] for k in ["page_size"]},
+            {k: reqparams[k] for k in ["short_name", "version", "page_size"]},
             {"provider": "NSIDC_CPRD"},
         )
 
@@ -264,7 +267,6 @@ def place_order(
         subsetparams,
         verbose,
         subset=True,
-        session=None,
         geom_filepath=None,
     ):  # , **kwargs):
         """
@@ -294,11 +296,6 @@ def place_order(
             Spatial subsetting returns all data that are within the area of interest
             (but not complete granules.
             This eliminates false-positive granules returned by the metadata-level search)
-        session : requests.session object
-            A session object authenticating the user to order data using their
-            Earthdata login information.
-            The session object will automatically be passed from the query object if you
-            have successfully logged in there.
         geom_filepath : string, default None
             String of the full filename and path when the spatial input is a file.
 
@@ -312,11 +309,6 @@ def place_order(
         query.Query.order_granules
         """
 
-        if session is None:
-            raise ValueError(
-                "Don't forget to log in to Earthdata using query.earthdata_login()"
-            )
-
         base_url = "https://n5eil02u.ecs.nsidc.org/egi/request"
 
         self.get_avail(CMRparams, reqparams)
@@ -352,15 +344,12 @@ def place_order(
                 total_pages,
                 " is submitting to NSIDC",
             )
-            request_params = apifmt.combine_params(CMRparams, reqparams, subsetparams)
             request_params.update({"page_num": page_num})
 
-            # DevNote: earlier versions of the code used a file upload+post rather than putting the geometries
-            # into the parameter dictionaries. However, this wasn't working with shapefiles, but this more general
-            # solution does, so the geospatial parameters are included in the parameter dictionaries.
-            request = session.get(base_url, params=request_params)
+            request = self.session.get(base_url, params=request_params)
 
-            # DevGoal: use the request response/number to do some error handling/give the user better messaging for failures
+            # DevGoal: use the request response/number to do some error handling/
+            # give the user better messaging for failures
             # print(request.content)
             root = ET.fromstring(request.content)
             # print([subset_agent.attrib for subset_agent in root.iter('SubsetAgent')])
@@ -394,7 +383,7 @@ def place_order(
                 print("status URL: ", statusURL)
 
             # Find order status
-            request_response = session.get(statusURL)
+            request_response = self.session.get(statusURL)
             if verbose is True:
                 print(
                     "HTTP response from order response URL: ",
@@ -419,7 +408,7 @@ def place_order(
                 )
                 # print('Status is not complete. Trying again')
                 time.sleep(10)
-                loop_response = session.get(statusURL)
+                loop_response = self.session.get(statusURL)
 
                 # Raise bad request: Loop will stop for bad response code.
                 loop_response.raise_for_status()
@@ -473,7 +462,7 @@ def place_order(
 
         return self.orderIDs
 
-    def download(self, verbose, path, session=None, restart=False):
+    def download(self, verbose, path, restart=False):
         """
         Downloads the data for the object's orderIDs, which are generated by ordering data
         from the NSIDC.
@@ -485,10 +474,6 @@ def download(self, verbose, path, session=None, restart=False):
             Progress information is automatically printed regardless of the value of verbose.
         path : string
             String with complete path to desired download directory and location.
-        session : requests.session object
-            A session object authenticating the user to download data using their Earthdata login information.
-            The session object will automatically be passed from the query object if you
-            have successfully logged in there.
         restart : boolean, default False
             Restart your download if it has been interrupted.
             If the kernel has been restarted, but you successfully
@@ -509,13 +494,6 @@ def download(self, verbose, path, session=None, restart=False):
             Unzip the downloaded granules.
         """
 
-        # Note: need to test these checks still
-        if session is None:
-            raise ValueError(
-                "Don't forget to log in to Earthdata using query.earthdata_login()"
-            )
-            # DevGoal: make this a more robust check for an active session
-
         # DevNote: this will replace any existing orderIDs with the saved list
         # (could create confusion depending on whether download was interrupted or kernel restarted)
         order_fn = ".order_restart"
@@ -529,7 +507,8 @@ def download(self, verbose, path, session=None, restart=False):
                 "Please confirm that you have submitted a valid order and it has successfully completed."
             )
 
-        # DevNote: Temporary. Hard code the orderID info files here. order_fn should be consistent with place_order.
+        # DevNote: Temporary. Hard code the orderID info files here.
+        # order_fn should be consistent with place_order.
 
         downid_fn = ".download_ID"
 
@@ -552,7 +531,7 @@ def download(self, verbose, path, session=None, restart=False):
             print("Beginning download of zipped output...")
 
             try:
-                zip_response = session.get(downloadURL)
+                zip_response = self.session.get(downloadURL)
                 # Raise bad request: Loop will stop for bad response code.
                 zip_response.raise_for_status()
                 print(
@@ -566,7 +545,8 @@ def download(self, verbose, path, session=None, restart=False):
                 print(
                     "Unable to download ", order, ". Check granule order for messages."
                 )
-            # DevGoal: move this option back out to the is2class level and implement it in an alternate way?
+            # DevGoal: move this option back out to the is2class level
+            # and implement it in an alternate way?
             #         #Note: extract the data to save it locally
             else:
                 with zipfile.ZipFile(io.BytesIO(zip_response.content)) as z:
-Original file line number
+Diff line change
@@ Expand Up / @@ -3,5 +3,4 @@ branch = True @@
     source = icepyx
     omit =
         setup.py
-        test/*
         doc/*