Ruff proposal

.github/workflows/static-analysis.yml

@@ -9,4 +9,4 @@ jobs:
 
   call-ruff-workflow:
     # Docs: https://github.com/ASFHyP3/actions
-    uses: ASFHyP3/actions/.github/workflows/reusable-ruff.yml@ruff
+    uses: ASFHyP3/actions/.github/workflows/reusable-ruff.yml@ruff-ruff

CHANGELOG.md

@@ -6,16 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [PEP 440](https://www.python.org/dev/peps/pep-0440/)
 and uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [0.8.3]
 
+## [0.8.2]
 ### Added
 * Ruff configuration and GitHub Action for Ruff-based static analysis
 
 ### Changed
-* Reformatted entire repository with the commands `ruff check --select I --fix .` and `ruff format .`
-
-## [0.8.2]
-### Changed
+* Reformatted the entire repository with the commands `ruff check --fix .` and `ruff format .` Linting issues that weren't automatically fixed were suppressed with `noqa` following [Ruff's recommendation](https://docs.astral.sh/ruff/tutorial/#adding-rules) when adding rules to an existing code base so that we can begin enforcing rules for new changes immediately.
 * All the special ISCE2 environment variable, python path, and system path handling has been moved to `hyp3_isce2.__init__.py` to ensure it's always done before using any object in this package.
 * All [subprocess](https://docs.python.org/3/library/subprocess.html#using-the-subprocess-module) calls use `subprocess.run`, as recommended.
 

pyproject.toml

@@ -75,6 +75,17 @@ src = ["src", "tests"]
 indent-style = "space"
 quote-style = "single"
 
+[tool.ruff.lint]
+extend-select = [
+    "UP",  # pyupgrade
+    "D",   # pydocstyle
+    "ANN", # annotations
+    "PTH", # use-pathlib-pth
+]
+
+[tool.ruff.lint.pydocstyle]
+convention = "google"
+
 [tool.ruff.lint.isort]
 case-sensitive = true
 lines-after-imports = 2

src/hyp3_isce2/__main__.py

@@ -1,6 +1,5 @@
-"""
-ISCE2 processing for HyP3
+"""ISCE2 processing for HyP3
 """
 import argparse
 import os
 import sys
@@ -11,11 +10,11 @@
 from hyp3lib.fetch import write_credentials_to_netrc_file
 
 
 def main():
     """Main entrypoint for HyP3 processing
 
     Calls the HyP3 entrypoint specified by the `++process` argument
     """
     parser = argparse.ArgumentParser(prefix_chars='+', formatter_class=argparse.ArgumentDefaultsHelpFormatter)
     parser.add_argument(
         '++process',

src/hyp3_isce2/burst.py

@@ -1,4 +1,4 @@
 import copy
 import logging
 import re
 import shutil
@@ -35,7 +35,7 @@
 class BurstMetadata:
     """Metadata for a burst."""
 
     def __init__(self, metadata: etree.Element, burst_params: BurstParams):
         self.safe_name = burst_params.granule
         self.swath = burst_params.swath
         self.polarization = burst_params.polarization
@@ -147,7 +147,7 @@
     if not out_file:
         return metadata
 
     with open(out_file, 'wb') as f:
         f.write(content)
 
     return str(out_file)
@@ -177,7 +177,7 @@
     return Path(out_file)
 
 
-def spoof_safe(burst: BurstMetadata, burst_tiff_path: Path, base_path: Path = Path('.')) -> Path:
+def spoof_safe(burst: BurstMetadata, burst_tiff_path: Path, base_path: Path = Path.cwd()) -> Path:
     """Spoof a Sentinel-1 SAFE file for a burst.
 
     The created SAFE file will be saved to the base_path directory. The SAFE will have the following structure:
@@ -223,12 +223,12 @@
     """Get the bounding box of a Sentinel-1 burst using ISCE2.
     Using ISCE2 directly ensures that the bounding box is the same as the one used by ISCE2 for processing.
 
-    args:
+    Args:
         params: The burst parameters.
         base_dir: The directory containing the SAFE file.
             If base_dir is not set, it will default to the current working directory.
 
-    returns:
+    Returns:
         The bounding box of the burst as a shapely.geometry.Polygon object.
     """
     if base_dir is None:
@@ -339,7 +339,6 @@
     Returns:
         The name of the interferogram product.
     """
-
     reference_split = reference_scene.split('_')
     secondary_split = secondary_scene.split('_')
 

src/hyp3_isce2/dem.py

@@ -60,6 +60,7 @@ def distance_meters_to_degrees(distance_meters, latitude):
     Args:
         distance_meters: Arc length in meters.
         latitude: The line of latitude at which the calculation takes place.
+
     Returns:
         The length in degrees for longitude and latitude, respectively.
     """
@@ -89,10 +90,11 @@ def download_dem_for_isce2(
         buffer: The extent buffer in degrees, by default .4, which is about 44 km at the equator
                 (or about 2.5 bursts at the equator).
         resample_20m: Whether or not the DEM should be resampled to 20 meters.
+
     Returns:
         The path to the downloaded DEM.
     """
-    dem_dir = dem_dir or Path('.')
+    dem_dir = dem_dir or Path.cwd()
     dem_dir.mkdir(exist_ok=True, parents=True)
 
     extent_buffered = buffer_extent(extent, buffer)

src/hyp3_isce2/insar_stripmap.py

@@ -1,5 +1,4 @@
-"""
-ISCE2 stripmap processing
+"""ISCE2 stripmap processing
 """
 
 import argparse
@@ -99,8 +98,7 @@ def get_product_file(product: asf_search.ASFProduct, file_prefix: str) -> str:
 
 
 def main():
-    """ Entrypoint for the stripmap workflow"""
-
+    """Entrypoint for the stripmap workflow"""
     parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.ArgumentDefaultsHelpFormatter)
 
     parser.add_argument('--bucket', help='AWS S3 bucket HyP3 for upload the final product(s)')

src/hyp3_isce2/insar_tops.py

@@ -98,7 +98,7 @@ def main():
 
     log.info('Begin ISCE2 TopsApp run')
 
-    range_looks, azimuth_looks = [int(looks) for looks in args.looks.split('x')]
+    range_looks, azimuth_looks = (int(looks) for looks in args.looks.split('x'))
     isce_output_dir = insar_tops(
         reference_scene=args.reference_scene,
         secondary_scene=args.secondary_scene,

src/hyp3_isce2/insar_tops_burst.py

@@ -206,7 +206,7 @@ def make_parameter_file(
         dem_name: Name of the DEM that is use
         dem_resolution: Resolution of the DEM
 
-    returns:
+    Returns:
         None
     """
     SPEED_OF_LIGHT = 299792458.0
@@ -298,7 +298,6 @@ def translate_outputs(isce_output_dir: Path, product_name: str, pixel_size: floa
         product_name: Name of the product
         pixel_size: Pixel size
     """
-
     src_ds = gdal.Open(str(isce_output_dir / 'filt_topophase.unw.geo'))
     src_geotransform = src_ds.GetGeoTransform()
     src_projection = src_ds.GetProjection()
@@ -433,7 +432,7 @@ def main():
     reference_scene, secondary_scene = oldest_granule_first(args.granules[0], args.granules[1])
     validate_bursts(reference_scene, secondary_scene)
     swath_number = int(reference_scene[12])
-    range_looks, azimuth_looks = [int(looks) for looks in args.looks.split('x')]
+    range_looks, azimuth_looks = (int(looks) for looks in args.looks.split('x'))
     apply_water_mask = args.apply_water_mask
 
     isce_output_dir = insar_tops_burst(

src/hyp3_isce2/stripmapapp_alos.py

@@ -87,7 +87,7 @@ def generate_template(self) -> str:
         Returns:
             The rendered template
         """
-        with open(TEMPLATE_DIR / 'stripmapapp_alos.xml', 'r') as file:
+        with open(TEMPLATE_DIR / 'stripmapapp_alos.xml') as file:
             template = Template(file.read())
         return template.render(self.__dict__)
 
@@ -123,7 +123,7 @@ def run_stripmapapp(dostep: str = '', start: str = '', end: str = '', config_xml
         ValueError: If the step is not a valid step (see TOPSAPP_STEPS)
     """
     if not config_xml.exists():
-        raise IOError(f'The config file {config_xml} does not exist!')
+        raise OSError(f'The config file {config_xml} does not exist!')
 
     if dostep and (start or end):
         raise ValueError('If dostep is specified, start and stop cannot be used')

src/hyp3_isce2/topsapp.py

@@ -90,7 +90,7 @@ def generate_template(self) -> str:
         Returns:
             The rendered template
         """
-        with open(TEMPLATE_DIR / 'topsapp.xml', 'r') as file:
+        with open(TEMPLATE_DIR / 'topsapp.xml') as file:
             template = Template(file.read())
         return template.render(self.__dict__)
 
@@ -149,7 +149,7 @@ def run_topsapp_burst(dostep: str = '', start: str = '', end: str = '', config_x
         ValueError: If the step is not a valid step (see TOPSAPP_STEPS)
     """
     if not config_xml.exists():
-        raise IOError(f'The config file {config_xml} does not exist!')
+        raise OSError(f'The config file {config_xml} does not exist!')
 
     if dostep and (start or end):
         raise ValueError('If dostep is specified, start and stop cannot be used')

src/hyp3_isce2/utils.py

@@ -14,7 +14,8 @@ class GDALConfigManager:
     """Context manager for setting GDAL config options temporarily"""
 
     def __init__(self, **options):
-        """
+        """Initialize the GDAL config options
+
         Args:
             **options: GDAL Config `option=value` keyword arguments.
         """
@@ -90,7 +91,7 @@ def oldest_granule_first(g1, g2):
 
 
 def load_isce2_image(in_path) -> tuple[isceobj.Image, np.ndarray]:
-    """ Read an ISCE2 image file and return the image object and array.
+    """Read an ISCE2 image file and return the image object and array.
 
     Args:
         in_path: The path to the image to resample (not the xml).
@@ -105,7 +106,7 @@ def load_isce2_image(in_path) -> tuple[isceobj.Image, np.ndarray]:
 
 
 def write_isce2_image(output_path, array=None, width=None, mode='read', data_type='FLOAT') -> None:
-    """ Write an ISCE2 image file.
+    """Write an ISCE2 image file.
 
     Args:
         output_path: The path to the output image file.
@@ -164,7 +165,6 @@ def resample_to_radar(
     Returns:
         resampled_image: The resampled image array
     """
-
     start_lon, delta_lon, start_lat, delta_lat = geotransform[0], geotransform[1], geotransform[3], geotransform[5]
 
     lati = np.clip((((lat - start_lat) / delta_lat) + 0.5).astype(int), 0, mask.shape[0] - 1)

tests/test_burst.py

@@ -44,8 +44,7 @@ def test_spoof_safe(tmp_path, mocker, pattern):
 
 @pytest.mark.parametrize('orbit', ('ascending', 'descending'))
 def test_get_region_of_interest(tmp_path, orbit):
-    """
-    Test that the region of interest is correctly calculated for a given burst pair.
+    """Test that the region of interest is correctly calculated for a given burst pair.
     Specifically, the region of interest we create should intersect the bursts used to create it,
     but not the bursts before or after it. This is difficult due to to the high degree of overlap between bursts.
 

tests/test_s1_auxcal.py

@@ -5,7 +5,6 @@ def test_download_aux_cal(tmp_path):
     """This test is slow because it download ~3MB of data.
     Might want to consider skipping unless doing integration testing.
     """
-
     aux_cal_dir = tmp_path / 'aux_cal'
     s1_auxcal.download_aux_cal(aux_cal_dir)
     assert (aux_cal_dir / 'S1A_AUX_CAL_V20190228T092500_G20210104T141310.SAFE').exists()

tests/test_stripmapapp.py

@@ -17,7 +17,7 @@ def test_stripmap_config(tmp_path):
     config.write_template(template_path)
     assert template_path.exists()
 
-    with open(template_path, 'r') as template_file:
+    with open(template_path) as template_file:
         template = template_file.read()
         assert 'ALPSRP156121200-L1.0/IMG-HH-ALPSRP156121200-H1.0__A' in template
         assert 'ALPSRP156121200-L1.0/LED-ALPSRP156121200-H1.0__A' in template

tests/test_topsapp.py

@@ -19,7 +19,7 @@ def test_topsapp_burst_config(tmp_path):
     config.write_template(template_path)
     assert template_path.exists()
 
-    with open(template_path, 'r') as template_file:
+    with open(template_path) as template_file:
         template = template_file.read()
         assert 'S1A_IW_SLC__1SDV_20200604T022251_20200604T022318_032861_03CE65_7C85.SAFE' in template
         assert 'S1A_IW_SLC__1SDV_20200616T022252_20200616T022319_033036_03D3A3_5D11.SAFE' in template