Merge pull request #672 from garlic-os/linting

Linting RAiDER codebase

CHANGELOG.md

@@ -6,7 +6,10 @@ 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).
 
-## [Unreleased]
+[Unreleased]
+### Changed
+* [672](https://github.com/dbekaert/RAiDER/pull/672) - Linted the project with `ruff`.
+
 ### Fixed
 * [679](https://github.com/dbekaert/RAiDER/pull/679) - Fixed a bug causing test_updateTrue to falsely pass.
 

CONTRIBUTING.md

@@ -111,6 +111,22 @@ git commit -a -m "Put here the synthetic commit message"
 git push my_user_name my_new_feature_branch
 ```
 
+### Formatting and linting with [Ruff](https://docs.astral.sh/ruff/) ###
+
+Format your code to follow the style of the project with:
+```
+ruff format
+```
+and check for linting problems with:
+```
+ruff check
+```
+Please ensure that any linting problems in your changes are resolved before
+submitting a pull request.
+> [!TIP]
+> vscode users can [install the ruff extension](https://marketplace.visualstudio.com/items?itemName=charliermarsh.ruff) to run the linter automatically in the
+editor.
+
 ### Issue a pull request from GitHub UI ###
 commit locally and push. To get a reasonable history, you may need to
 

pyproject.toml

@@ -66,3 +66,28 @@ multi_line_output = 5
 default_section = "THIRDPARTY"
 
 [tool.setuptools_scm]
+
+[tool.ruff]
+line-length = 120
+src = ["tools", "test"]
+
+[tool.ruff.format]
+indent-style = "space"
+quote-style = "single"
+
+[tool.ruff.lint]
+extend-select = [
+    "I",   # isort: https://docs.astral.sh/ruff/rules/#isort-i
+    "UP",  # pyupgrade: https://docs.astral.sh/ruff/rules/#pyupgrade-up
+    "D",   # pydocstyle: https://docs.astral.sh/ruff/rules/#pydocstyle-d
+    "ANN", # annotations: https://docs.astral.sh/ruff/rules/#flake8-annotations-ann
+    "PTH", # use-pathlib-pth: https://docs.astral.sh/ruff/rules/#flake8-use-pathlib-pth
+]
+ignore = ["ANN101", "D200", "D205", "D212"]
+
+[tool.ruff.lint.pydocstyle]
+convention = "google"
+
+[tool.ruff.lint.isort]
+case-sensitive = true
+lines-after-imports = 2

setup.py

@@ -5,7 +5,6 @@
 # RESERVED. United States Government Sponsorship acknowledged.
 #
 # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-import re
 from pathlib import Path
 
 import numpy as np

test/__init__.py

@@ -1,7 +1,4 @@
 import os
-import pytest
-import subprocess
-import shutil
 import string
 import random
 from contextlib import contextmanager
@@ -31,37 +28,6 @@ def pushd(dir):
     os.chdir(prevdir)
 
 
-def update_yaml(dct_cfg:dict, dst:str='temp.yaml'):
-    """ Write a new yaml file from a dictionary.
-
-    Updates parameters in the default 'template.yaml' file.
-    Each key:value pair will in 'dct_cfg' will overwrite that in the default
-    """
-    import RAiDER, yaml
-
-    run_config_path = os.path.join(
-        os.path.dirname(RAiDER.__file__),
-        'cli',
-        'examples',
-        'template',
-        'template.yaml'
-    )
-
-    with open(run_config_path, 'r') as f:
-        try:
-            params = yaml.safe_load(f)
-        except yaml.YAMLError as exc:
-            print(exc)
-            raise ValueError(f'Something is wrong with the yaml file {run_config_path}')
-
-    params = {**params, **dct_cfg}
-
-    with open(dst, 'w') as fh:
-        yaml.safe_dump(params, fh, default_flow_style=False)
-
-    return dst
-
-
 def makeLatLonGrid(bbox, reg, out_dir, spacing=0.1):
     """ Make lat lons at a specified spacing """
     S, N, W, E = bbox

test/_scenario_1.py

@@ -10,7 +10,7 @@
 from RAiDER.delay import main
 from RAiDER.utilFcns import rio_open
 from RAiDER.checkArgs import makeDelayFileNames
-from RAiDER.cli.validators import modelName2Module
+from RAiDER.cli.validators import get_wm_by_name
 
 SCENARIO_DIR = os.path.join(TEST_DIR, "scenario_1")
 _RTOL = 1e-2
@@ -93,7 +93,7 @@ def core_test_tropo_delay(tmp_path, modelName):
     if not os.path.exists(wmLoc):
         os.mkdir(wmLoc)
 
-    _, model_obj = modelName2Module(modelName)
+    _, model_obj = get_wm_by_name(modelName)
     wet_file, hydro_file = makeDelayFileNames(
         time, Zenith, "envi", modelName, tmp_path
     )

test/_scenario_2.py

@@ -9,7 +9,7 @@
 
 from RAiDER.delay import main
 from RAiDER.losreader import Zenith
-from RAiDER.cli.validators import modelName2Module
+from RAiDER.cli.validators import get_wm_by_name
 
 SCENARIO_DIR = os.path.join(TEST_DIR, "scenario_2")
 _RTOL = 1e-2
@@ -34,7 +34,7 @@ def test_computeDelay(tmp_path):
     lats = stats['Lat'].values
     lons = stats['Lon'].values
 
-    _, model_obj = modelName2Module('ERA5')
+    _, model_obj = get_wm_by_name('ERA5')
 
     with pushd(tmp_path):
 

test/scenario_1/raider_example_1.yaml

@@ -11,7 +11,3 @@
   height_group:
     height_levels: 0 50 100 500 1000 # Return only these specific height levels
   los_group: # absent other options ZTD is calculated
-  runtime_group:
-    output_directory: test/scenario_1
-
-

test/test_GUNW.py

@@ -18,6 +18,7 @@
 import RAiDER.cli.raider as raider
 import RAiDER.s1_azimuth_timing
 from RAiDER import aws
+import RAiDER.aria.prepFromGUNW
 from RAiDER.aria.prepFromGUNW import (
     check_hrrr_dataset_availablity_for_s1_azimuth_time_interpolation,
     check_weather_model_availability,_get_acq_time_from_gunw_id,
@@ -369,6 +370,18 @@ def test_check_weather_model_availability_over_alaska(test_gunw_path_factory, we
     assert cond
 
 
+@pytest.mark.parametrize('weather_model_name', ['ERA5', 'GMAO', 'MERRA2', 'HRRR'])
+def test_check_weather_model_availability_2(weather_model_name):
+    gunw_id = Path("test/gunw_test_data/S1-GUNW-D-R-059-tops-20230320_20220418-180300-00179W_00051N-PP-c92e-v2_0_6.nc")
+    assert check_weather_model_availability(gunw_id, weather_model_name)
+
+
+def test_check_weather_model_availability_3():
+    gunw_id = Path("test/gunw_test_data/S1-GUNW-D-R-059-tops-20230320_20220418-180300-00179W_00051N-PP-c92e-v2_0_6.nc")
+    with pytest.raises(ValueError):
+        check_weather_model_availability(gunw_id, 'NotAModel')
+
+
 @pytest.mark.parametrize('weather_model_name', ['HRRR'])
 @pytest.mark.parametrize('location', ['california-t71', 'alaska'])
 def test_weather_model_availability_integration_using_valid_range(location,
@@ -509,7 +522,7 @@ def test_hyp3_exits_succesfully_when_hrrr_not_available(mocker):
                  side_effect=[False])
     # The gunw id should not have a hyp3 file associated with it
     # This call will still hit the HRRR s3 API as done in the previous test
-    mocker.patch("RAiDER.aws.get_s3_file", side_effect=['hyp3-job-uuid-3ad24/S1-GUNW-A-R-106-tops-20160809_20140101-160001-00078W_00041N-PP-4be8-v3_0_0.nc'])
+    mocker.patch("RAiDER.aws.get_s3_file", side_effect=[Path('hyp3-job-uuid-3ad24/S1-GUNW-A-R-106-tops-20160809_20140101-160001-00078W_00041N-PP-4be8-v3_0_0.nc')])
     mocker.patch('RAiDER.aria.prepFromGUNW.check_weather_model_availability')
     iargs = [
                '--bucket', 's3://foo',
@@ -622,22 +635,21 @@ def test_check_hrrr_availability_all_true():
     gunw_id = "S1-GUNW-A-R-106-tops-20220115_20211222-225947-00078W_00041N-PP-4be8-v3_0_0"
 
     # Mock _get_acq_time_from_gunw_id to return expected times
-    result = check_hrrr_dataset_availablity_for_s1_azimuth_time_interpolation(gunw_id)
-    assert result == True
+    assert check_hrrr_dataset_availablity_for_s1_azimuth_time_interpolation(gunw_id)
 
 def test_get_slc_ids_from_gunw():
-    test_path = 'test/gunw_test_data/S1-GUNW-D-R-059-tops-20230320_20220418-180300-00179W_00051N-PP-c92e-v2_0_6.nc'
+    test_path = Path('test/gunw_test_data/S1-GUNW-D-R-059-tops-20230320_20220418-180300-00179W_00051N-PP-c92e-v2_0_6.nc')
     assert get_slc_ids_from_gunw(test_path, 'reference') == 'S1A_IW_SLC__1SDV_20230320T180251_20230320T180309_047731_05BBDB_DCA0.zip'
     assert get_slc_ids_from_gunw(test_path, 'secondary') == 'S1A_IW_SLC__1SDV_20220418T180246_20220418T180305_042831_051CC3_3C47.zip'
 
     with pytest.raises(FileNotFoundError):
-        get_slc_ids_from_gunw('dummy.nc')
+        get_slc_ids_from_gunw(Path('dummy.nc'))
 
     with pytest.raises(ValueError):
         get_slc_ids_from_gunw(test_path, 'tertiary')
 
     with pytest.raises(OSError):
-        get_slc_ids_from_gunw('test/weather_files/ERA-5_2020_01_30_T13_52_45_32N_35N_120W_115W.nc')
+        get_slc_ids_from_gunw(Path('test/weather_files/ERA-5_2020_01_30_T13_52_45_32N_35N_120W_115W.nc'))
 
 
 def test_get_acq_time_valid_slc_id():
@@ -653,20 +665,3 @@ def test_get_acq_time_invalid_slc_id():
   invalid_slc_id = "test/gunw_azimuth_test_data/S1B_OPER_AUX_POEORB_OPOD_20210731T111940_V20210710T225942_20210712T005942.EOF"
   with pytest.raises(ValueError):
     get_acq_time_from_slc_id(invalid_slc_id)
-
-
-def test_check_weather_model_availability():
-    gunw_id = "test/gunw_test_data/S1-GUNW-D-R-059-tops-20230320_20220418-180300-00179W_00051N-PP-c92e-v2_0_6.nc"
-    weather_models = ['ERA5', 'GMAO', 'MERRA2', 'HRRR']
-    for wm in weather_models:
-        assert check_weather_model_availability(gunw_id, wm)
-
-    with pytest.raises(ValueError):
-        check_weather_model_availability(gunw_id, 'NotAModel')
-
-def test_check_weather_model_availability_2():
-    gunw_id = "test/gunw_test_data/S1-GUNW-D-R-059-tops-20230320_20220418-180300-00179W_00051N-PP-c92e-v2_0_6.nc"
-    weather_models = ['ERA5', 'GMAO', 'MERRA2', 'HRRR']
-    fail_check = [True, True, True, True]
-    for wm, check in zip(weather_models, fail_check):
-        assert check_weather_model_availability(gunw_id, wm)==check

test/test_HRRR_ztd.py

@@ -1,42 +1,21 @@
-import os
-import subprocess
-import shutil
-import glob
-
-from test import TEST_DIR, WM, update_yaml, pushd
+from test import TEST_DIR, pushd
 
 import numpy as np
 import xarray as xr
 from RAiDER.cli.raider import calcDelays
 
-
 def test_scenario_1(tmp_path, data_for_hrrr_ztd, mocker):
+    SCENARIO_DIR = TEST_DIR / "scenario_1"
+    test_path = SCENARIO_DIR / 'raider_example_1.yaml'
+    mocker.patch('RAiDER.processWM.prepareWeatherModel',
+                    side_effect=[str(data_for_hrrr_ztd)])
+
     with pushd(tmp_path):
-        dct_group = {
-            "aoi_group": {"bounding_box": [36, 37, -92, -91]},
-            "date_group": {"date_start": "20200101"},
-            "time_group": {"time": "12:00:00", "interpolate_time": "none"},
-            "weather_model": "HRRR",
-            "height_group": {"height_levels": [0, 50, 100, 500, 1000]},
-            "look_dir": "right",
-            "runtime_group": {"output_directory": "test/scenario_1"},
-        }
-
-        cfg = update_yaml(dct_group, os.path.join(tmp_path, "temp.yaml"))
-
-        SCENARIO_DIR = os.path.join(tmp_path, TEST_DIR, "scenario_1")
-        mocker.patch(
-            "RAiDER.processWM.prepareWeatherModel", side_effect=[str(data_for_hrrr_ztd)]
-        )
-        calcDelays([os.path.join(tmp_path, "temp.yaml")])
+        calcDelays([str(test_path)])
+        new_data  = xr.load_dataset('HRRR_tropo_20200101T120000_ztd.nc')
 
-        new_data = xr.load_dataset(
-            os.path.join(
-                tmp_path, "test", "scenario_1", "HRRR_tropo_20200101T120000_ztd.nc"
-            )
-        )
-        new_data1 = new_data.sel(x=-91.84, y=36.84, z=0, method="nearest")
-        golden_data = 2.2622863, 0.0361021  # hydro|wet
+    new_data1 = new_data.sel(x=-91.84, y=36.84, z=0, method='nearest')
+    golden_data = 2.2622863, 0.0361021 # hydro|wet
 
-        np.testing.assert_almost_equal(golden_data[0], new_data1["hydro"].data)
-        np.testing.assert_almost_equal(golden_data[1], new_data1["wet"].data)
+    np.testing.assert_almost_equal(golden_data[0], new_data1["hydro"].data)
+    np.testing.assert_almost_equal(golden_data[1], new_data1["wet"].data)