Merge pull request #246 from ASFHyP3/review

Update based on release review

CHANGELOG.md

@@ -12,6 +12,7 @@ and uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Changed
 - The interface for `insar_tops_burst` so that it takes `--reference` and `--secondary` granule lists. The positional `granules` argument is now optional and deprecated.
+- Moved HyP3 product packaging functionality out of `insar_tops_burst.py` and to a new `packaging.py` so that both `insar_tops` and `insar_tops_burst` can use it.
 
 ## [2.0.0]
 ### Changed

src/hyp3_isce2/insar_tops.py

@@ -63,7 +63,7 @@ def insar_tops(
         orbit_file = fetch_for_scene(granule, dir=orbit_dir)
         log.info(f'Got orbit file {orbit_file} from s1_orbits')
 
-    config = topsapp.TopsappBurstConfig(
+    config = topsapp.TopsappConfig(
         reference_safe=f'{reference_scene}.SAFE',
         secondary_safe=f'{secondary_scene}.SAFE',
         polarization=polarization,
@@ -79,20 +79,20 @@ def insar_tops(
     config_path = config.write_template('topsApp.xml')
 
     if apply_water_mask:
-        topsapp.run_topsapp_burst(start='startup', end='filter', config_xml=config_path)
+        topsapp.run_topsapp(start='startup', end='filter', config_xml=config_path)
         water_mask_path = 'water_mask.wgs84'
         create_water_mask(str(dem_path), water_mask_path)
         multilook('merged/lon.rdr.full', outname='merged/lon.rdr', alks=azimuth_looks, rlks=range_looks)
         multilook('merged/lat.rdr.full', outname='merged/lat.rdr', alks=azimuth_looks, rlks=range_looks)
         resample_to_radar_io(water_mask_path, 'merged/lat.rdr', 'merged/lon.rdr', 'merged/water_mask.rdr')
         isce2_copy('merged/phsig.cor', 'merged/unmasked.phsig.cor')
         image_math('merged/unmasked.phsig.cor', 'merged/water_mask.rdr', 'merged/phsig.cor', 'a*b')
-        topsapp.run_topsapp_burst(start='unwrap', end='unwrap2stage', config_xml=config_path)
+        topsapp.run_topsapp(start='unwrap', end='unwrap2stage', config_xml=config_path)
         isce2_copy('merged/unmasked.phsig.cor', 'merged/phsig.cor')
     else:
-        topsapp.run_topsapp_burst(start='startup', end='unwrap2stage', config_xml=config_path)
+        topsapp.run_topsapp(start='startup', end='unwrap2stage', config_xml=config_path)
     copyfile('merged/z.rdr.full.xml', 'merged/z.rdr.full.vrt.xml')
-    topsapp.run_topsapp_burst(start='geocode', end='geocode', config_xml=config_path)
+    topsapp.run_topsapp(start='geocode', end='geocode', config_xml=config_path)
 
     return Path('merged')
 

src/hyp3_isce2/insar_tops_burst.py

@@ -96,7 +96,7 @@ def insar_tops_burst(
         orbit_file = fetch_for_scene(granule, dir=orbit_dir)
         log.info(f'Got orbit file {orbit_file} from s1_orbits')
 
-    config = topsapp.TopsappBurstConfig(
+    config = topsapp.TopsappConfig(
         reference_safe=f'{ref_params.granule}.SAFE',
         secondary_safe=f'{sec_params.granule}.SAFE',
         polarization=ref_params.polarization,
@@ -111,23 +111,23 @@ def insar_tops_burst(
     )
     config_path = config.write_template('topsApp.xml')
 
-    topsapp.run_topsapp_burst(start='startup', end='preprocess', config_xml=config_path)
+    topsapp.run_topsapp(start='startup', end='preprocess', config_xml=config_path)
     topsapp.swap_burst_vrts()
     if apply_water_mask:
-        topsapp.run_topsapp_burst(start='computeBaselines', end='filter', config_xml=config_path)
+        topsapp.run_topsapp(start='computeBaselines', end='filter', config_xml=config_path)
         water_mask_path = 'water_mask.wgs84'
         create_water_mask(str(dem_path), water_mask_path)
         multilook('merged/lon.rdr.full', outname='merged/lon.rdr', alks=azimuth_looks, rlks=range_looks)
         multilook('merged/lat.rdr.full', outname='merged/lat.rdr', alks=azimuth_looks, rlks=range_looks)
         resample_to_radar_io(water_mask_path, 'merged/lat.rdr', 'merged/lon.rdr', 'merged/water_mask.rdr')
         isce2_copy('merged/phsig.cor', 'merged/unmasked.phsig.cor')
         image_math('merged/unmasked.phsig.cor', 'merged/water_mask.rdr', 'merged/phsig.cor', 'a*b')
-        topsapp.run_topsapp_burst(start='unwrap', end='unwrap2stage', config_xml=config_path)
+        topsapp.run_topsapp(start='unwrap', end='unwrap2stage', config_xml=config_path)
         isce2_copy('merged/unmasked.phsig.cor', 'merged/phsig.cor')
     else:
-        topsapp.run_topsapp_burst(start='computeBaselines', end='unwrap2stage', config_xml=config_path)
+        topsapp.run_topsapp(start='computeBaselines', end='unwrap2stage', config_xml=config_path)
     copyfile('merged/z.rdr.full.xml', 'merged/z.rdr.full.vrt.xml')
-    topsapp.run_topsapp_burst(start='geocode', end='geocode', config_xml=config_path)
+    topsapp.run_topsapp(start='geocode', end='geocode', config_xml=config_path)
 
     return Path('merged')
 

src/hyp3_isce2/topsapp.py

@@ -41,7 +41,7 @@
 ]
 
 
-class TopsappBurstConfig:
+class TopsappConfig:
     """Configuration for a topsApp.py run"""
 
     def __init__(
@@ -135,8 +135,8 @@ def swap_burst_vrts():
         del base
 
 
-def run_topsapp_burst(dostep: str = '', start: str = '', end: str = '', config_xml: Path = Path('topsApp.xml')):
-    """Run topsApp.py for a burst pair with the desired steps and config file
+def run_topsapp(dostep: str = '', start: str = '', end: str = '', config_xml: Path = Path('topsApp.xml')):
+    """Run topsApp.py for a granule pair with the desired steps and config file
 
     Args:
         dostep: The step to run

tests/test_topsapp.py

@@ -1,10 +1,10 @@
 import pytest
 
-from hyp3_isce2.topsapp import TopsappBurstConfig, run_topsapp_burst, swap_burst_vrts
+from hyp3_isce2.topsapp import TopsappConfig, run_topsapp, swap_burst_vrts
 
 
 def test_topsapp_burst_config(tmp_path):
-    config = TopsappBurstConfig(
+    config = TopsappConfig(
         reference_safe='S1A_IW_SLC__1SDV_20200604T022251_20200604T022318_032861_03CE65_7C85.SAFE',
         secondary_safe='S1A_IW_SLC__1SDV_20200616T022252_20200616T022319_033036_03D3A3_5D11.SAFE',
         polarization='VV',
@@ -49,9 +49,9 @@ def test_swap_burst_vrts(tmp_path, monkeypatch):
 
 def test_run_topsapp_burst(tmp_path, monkeypatch):
     with pytest.raises(IOError):
-        run_topsapp_burst('topsApp.xml')
+        run_topsapp('topsApp.xml')
 
-    config = TopsappBurstConfig(
+    config = TopsappConfig(
         reference_safe='',
         secondary_safe='',
         polarization='',
@@ -67,10 +67,10 @@ def test_run_topsapp_burst(tmp_path, monkeypatch):
     template_path = config.write_template(tmp_path / 'topsApp.xml')
 
     with pytest.raises(ValueError, match=r'.*not a valid step.*'):
-        run_topsapp_burst('notastep', config_xml=template_path)
+        run_topsapp('notastep', config_xml=template_path)
 
     with pytest.raises(ValueError, match=r'^If dostep is specified, start and stop cannot be used$'):
-        run_topsapp_burst('preprocess', 'startup', config_xml=template_path)
+        run_topsapp('preprocess', 'startup', config_xml=template_path)
 
     monkeypatch.chdir(tmp_path)
-    run_topsapp_burst('preprocess', config_xml=template_path)
+    run_topsapp('preprocess', config_xml=template_path)