Merge pull request #108 from GeoscienceAustralia/NPI-4443-unittest-cleanup

NPI-4443 Clean up unit test outputs and temporarily pin supported Pandas version

Author: treefern

gnssanalysis/filenames.py

@@ -134,7 +134,7 @@ def determine_file_name_main(
             else:
                 print(new_name)
         except NotImplementedError:
-            logging.warning(f"Skipping {f.name} as {f.suffix} files are not yet supported.")
+            warnings.warn(f"Skipping {f.name} as {f.suffix} files are not yet supported.")
 
 
 def determine_file_name(
@@ -542,8 +542,8 @@ def determine_clk_name_props(file_path: pathlib.Path) -> dict[str, Any]:
     except Exception as e:
         # TODO: Work out what exceptions read_clk can actually throw when given a non-CLK file
         # At the moment we will also swallow errors we really shouldn't
-        logging.warning(f"{file_path.name} can't be read as an CLK file. Defaulting properties.")
-        logging.warning(f"Exception {e}, {type(e)}")
+        warnings.warn(f"{file_path.name} can't be read as an CLK file. Defaulting properties.")
+        warnings.warn(f"Exception {e}, {type(e)}")
         logging.info(traceback.format_exc())
         return {}
     return name_props
@@ -597,8 +597,8 @@ def determine_erp_name_props(file_path: pathlib.Path) -> dict[str, Any]:
     except Exception as e:
         # TODO: Work out what exceptions read_erp can actually throw when given a non-ERP file
         # At the moment we will also swallow errors we really shouldn't
-        logging.warning(f"{file_path.name} can't be read as an ERP file. Defaulting properties.")
-        logging.warning(f"Exception {e}, {type(e)}")
+        warnings.warn(f"{file_path.name} can't be read as an ERP file. Defaulting properties.")
+        warnings.warn(f"Exception {e}, {type(e)}")
         logging.info(traceback.format_exc())
         return {}
     return name_props
@@ -689,8 +689,8 @@ def determine_snx_name_props(file_path: pathlib.Path) -> dict[str, Any]:
     except Exception as e:
         # TODO: Work out what exceptions _get_snx_vector can actually throw when given a non-SNX file
         # At the moment we will also swallow errors we really shouldn't
-        logging.warning(f"{file_path.name} can't be read as an SNX file. Defaulting properties.")
-        logging.warning(f"Exception {e}, {type(e)}")
+        warnings.warn(f"{file_path.name} can't be read as an SNX file. Defaulting properties.")
+        warnings.warn(f"Exception {e}, {type(e)}")
         logging.info(traceback.format_exc())
         return {}
     return name_props
@@ -1015,7 +1015,7 @@ def check_filename_and_contents_consistency(
     # If parsing of a long filename fails, Project will not be present. In this case we have with minimal (and
     # maybe incorrect) properties to compare. So we raise a warning.
     if "project" not in file_name_properties:
-        logging.warning(
+        warnings.warn(
             f"Failed to parse filename according to the long filename format: '{input_file.name}'. "
             "As a result few useful properties are available to compare with the file contents, so the "
             "detailed consistency check will be skipped!"
@@ -1027,17 +1027,19 @@ def check_filename_and_contents_consistency(
 
     contents_epoch_interval = file_content_properties.get("sampling_rate_seconds", None)
     if contents_epoch_interval is None:
-        logging.warning(
+        warnings.warn(
             f"Sampling rate couldn't be inferred from file contents '{input_file.name}'. "
             "Cannot allow for timespan discrepancies of one epoch interval, so an error may follow."
         )
 
     discrepancies = {}
     # Check for keys only present on one side
     orphan_keys = set(file_name_properties.keys()).symmetric_difference((set(file_content_properties.keys())))
-    logging.warning(
+    orphan_keys_sorted = list(orphan_keys)
+    orphan_keys_sorted.sort()
+    warnings.warn(
         "The following properties can't be compared, as they were extracted only from file content or "
-        f"name (not both): {str(orphan_keys)}"
+        f"name (not both): {str(orphan_keys_sorted)}"
     )
     if output_orphan_prop_names:
         # Output properties found only in content OR filename.

gnssanalysis/gn_diffaux.py

@@ -1,6 +1,7 @@
 import logging as _logging
 from pathlib import Path as _Path
 from typing import Literal, Union
+import warnings
 
 import numpy as _np
 import pandas as _pd
@@ -118,7 +119,7 @@ def _compare_states(diffstd: _pd.DataFrame, log_lvl: int, tol: Union[float, None
     diff_states = diffstd.unstack(["TYPE", "SITE", "SAT", "BLK", "NUM"])
     # we remove the '.droplevel("NUM", axis=0)' due to ORBIT_PTS non-uniqueness. Changing to ORBIT_PTS_blah might be a better solution
     if diff_states.empty:
-        _logging.warning(msg=f":diffutil states not present. Skipping")
+        warnings.warn(f":diffutil states not present. Skipping")
         return 0
     if plot:
         # a standard scatter plot
@@ -157,7 +158,7 @@ def _compare_residuals(diffstd: _pd.DataFrame, log_lvl: int, tol: Union[float, N
     idx_names_to_unstack.remove("TIME")  # all but TIME: ['SITE', 'TYPE', 'SAT', 'NUM', 'It', 'BLK']
     diff_residuals = diffstd.unstack(idx_names_to_unstack)
     if diff_residuals.empty:
-        _logging.warning(f":diffutil residuals not present. Skipping")
+        warnings.warn(f":diffutil residuals not present. Skipping")
         return 0
     bad_residuals = _diff2msg(diff_residuals, tol=tol)
     if bad_residuals is not None:
@@ -176,7 +177,7 @@ def _compare_residuals(diffstd: _pd.DataFrame, log_lvl: int, tol: Union[float, N
 def _compare_stec(diffstd, log_lvl, tol=None):
     stec_diff = diffstd.unstack(level=("SITE", "SAT", "LAYER"))
     if stec_diff.empty:
-        _logging.warning(f":diffutil stec states not present. Skipping")
+        warnings.warn(f":diffutil stec states not present. Skipping")
         return 0
     bad_sv_states = _diff2msg(stec_diff, tol, dt_as_gpsweek=True)
     if bad_sv_states is not None:
@@ -323,7 +324,7 @@ def compare_clk(
     clk_b: baseline (normally b is test)
     """
 
-    _logging.warning(
+    warnings.warn(
         "compare_clk() is deprecated. Please use diff_clk() and note that the clk inputs are in opposite order"
     )
     return diff_clk(clk_baseline=clk_b, clk_test=clk_a, norm_types=norm_types, ext_dt=ext_dt, ext_svs=ext_svs)
@@ -428,7 +429,7 @@ def sisre(
     DEPRECATED
     """
 
-    _logging.warning(
+    warnings.warn(
         "sisre() is deprecated, please use calculate_sisre() instead. Note that input arg test/baseline orders are flipped"
     )
     return calculate_sisre(

gnssanalysis/gn_download.py

@@ -94,12 +94,11 @@ def __call__(self, bytes_transferred):
             _sys.stdout.flush()
 
 
-
-def get_earthdata_credentials(username: str = None, password: str = None) -> Tuple[str, str]:
+def get_earthdata_credentials(username: Optional[str] = None, password: Optional[str] = None) -> Tuple[str, str]:
     """
     Get NASA Earthdata credentials from .netrc file or direct parameters.
-    :param str username: Directly provided username (highest priority)
-    :param str password: Directly provided password (highest priority)
+    :param Optional[str] username: Directly provided username (highest priority)
+    :param Optional[str] password: Directly provided password (highest priority)
     :return Tuple[str, str]: Username and password tuple
     :raises ValueError: If no credentials can be obtained
     """
@@ -774,17 +773,17 @@ def ftp_tls(url: str, **kwargs) -> Generator[Any, Any, Any]:
 
 def download_file_from_cddis(
     filename: str,
-    ftp_folder: Optional[str] = None,     # deprecated
-    url_folder: Optional[str] = None,     # preferred
+    ftp_folder: Optional[str] = None,  # deprecated
+    url_folder: Optional[str] = None,  # preferred
     output_folder: _Path = _Path("."),
     max_retries: int = 3,
     decompress: bool = True,
     if_file_present: str = "prompt_user",
-    username: str = None,
-    password: str = None,
+    username: Optional[str] = None,
+    password: Optional[str] = None,
     note_filetype: Optional[str] = None,
 ) -> Union[_Path, None]:
-    """ Download a single file from the CDDIS HTTPS archive using NASA Earthdata authentication
+    """Download a single file from the CDDIS HTTPS archive using NASA Earthdata authentication
 
     :param str filename: Name of the file to download
     :param str ftp_folder: (Deprecated) Legacy folder path on the CDDIS FTP server. Use url_folder instead
@@ -794,8 +793,8 @@ def download_file_from_cddis(
     :param bool decompress: If true, decompresses files on download, defaults to True
     :param str if_file_present: What to do if file already present: "replace", "dont_replace", defaults to "prompt_user"
     :param str note_filetype: How to label the file for STDOUT messages, defaults to None
-    :param str username: NASA Earthdata username (optional, will try .netrc if not provided).
-    :param str password: NASA Earthdata password (optional, will try .netrc if not provided).
+    :param Optional[str] username: NASA Earthdata username (optional, will try .netrc if not provided).
+    :param Optional[str] password: NASA Earthdata password (optional, will try .netrc if not provided).
     :raises ValueError: If no credentials can be obtained.
     :raises requests.RequestException: If the file cannot be downloaded after retries.
     :return _Path or None: The pathlib.Path of the downloaded file (or decompressed output of it).
@@ -823,14 +822,8 @@ def download_file_from_cddis(
     if download_filepath is None:
         return None  # File exists and user chose not to replace
 
-    # Get NASA Earthdata credentials
-    try:
-        earthdata_username, earthdata_password = get_earthdata_credentials(
-            username=username, password=password
-        )
-    except ValueError as e:
-        logging.error(f"Failed to obtain NASA Earthdata credentials: {e}")
-        raise
+    # Get NASA Earthdata credentials (raises ValueError on failure)
+    earthdata_username, earthdata_password = get_earthdata_credentials(username=username, password=password)
 
     retries = 0
     while retries <= max_retries:
@@ -862,25 +855,27 @@ def download_file_from_cddis(
         except _requests.exceptions.RequestException as e:
             retries += 1
             if retries > max_retries:
+                # TODO consider wrapping the RequestException with this, and raising that, rather than logging an error
                 logging.error(f"Failed to download {filename} after {max_retries} retries: {e}")
                 if download_filepath.is_file():
                     download_filepath.unlink()
                 raise
             backoff = _random.uniform(0.0, 2.0 ** retries)
-            logging.warning(f"Error downloading {filename}: {e} "
-                            f"(retry {retries}/{max_retries}, backoff {backoff:.1f}s)")
+            _warnings.warn(
+                f"Error downloading {filename}: {e} " f"(retry {retries}/{max_retries}, backoff {backoff:.1f}s)"
+            )
             _time.sleep(backoff)
 
     raise Exception("Unexpected fallthrough in download_file_from_cddis.")
 
 
 def download_multiple_files_from_cddis(
     files: List[str],
-    ftp_folder: Optional[str] = None,     # deprecated
-    url_folder: Optional[str] = None,     # preferred
+    ftp_folder: Optional[str] = None,  # deprecated
+    url_folder: Optional[str] = None,  # preferred
     output_folder: _Path = _Path("."),
-    username: str = None,
-    password: str = None,
+    username: Optional[str] = None,
+    password: Optional[str] = None,
 ) -> None:
     """
     Download multiple files from the CDDIS HTTPS archive concurrently, using a thread pool.
@@ -889,8 +884,8 @@ def download_multiple_files_from_cddis(
     :param str ftp_folder: (Deprecated) Legacy folder path on the CDDIS FTP server. Use url_folder instead.
     :param str url_folder: Folder path (relative to CDDIS HTTPS archive root).
     :param _Path output_folder: Local folder to store the output files.
-    :param str username: NASA Earthdata username (optional, will try .netrc if not provided).
-    :param str password: NASA Earthdata password (optional, will try .netrc if not provided).
+    :param Optional[str] username: NASA Earthdata username (optional, will try .netrc if not provided).
+    :param Optional[str] password: NASA Earthdata password (optional, will try .netrc if not provided).
     :raises ValueError: If both ftp_folder and url_folder are provided.
     :return None: Runs downloads in parallel, does not return values. Each file is handled independently.
     """
@@ -943,8 +938,8 @@ def download_product_from_cddis(
     project_type: str = "OPS",
     timespan: _datetime.timedelta = _datetime.timedelta(days=2),
     if_file_present: str = "prompt_user",
-    username: str = None,
-    password: str = None,
+    username: Optional[str] = None,
+    password: Optional[str] = None,
 ) -> List[_Path]:
     """Download the file/s from CDDIS based on start and end epoch, to the download directory (download_dir)
 
@@ -961,8 +956,8 @@ def download_product_from_cddis(
     :param str project_type: Project type of file to download (e.g. ), defaults to "OPS"
     :param _datetime.timedelta timespan: Timespan of the file/s to download, defaults to _datetime.timedelta(days=2)
     :param str if_file_present: What to do if file already present: "replace", "dont_replace", defaults to "prompt_user"
-    :param str username: NASA Earthdata username (optional, will try .netrc if not provided).
-    :param str password: NASA Earthdata password (optional, will try .netrc if not provided).
+    :param Optional[str] username: NASA Earthdata username (optional, will try .netrc if not provided).
+    :param Optional[str] password: NASA Earthdata password (optional, will try .netrc if not provided).
     :raises FileNotFoundError: Raise error if the specified file cannot be found on CDDIS
     :raises Exception: If a file fails to download after all retries.
     :return List[_Path]: List of pathlib.Path objects to downloaded (or decompressed) files.