use radargrid step parameters to simplify _steps_to_vecs

[scottstanie]

Now that the function self.as_isce3_radargrid has parameters for step sizes, we dont need to redo the logic to compute the coordinates.

This should fix #105 where the differing logic causes different coordinates.

It also allows us to remove redundant error checking that happens in the as_isce3_radaragrid function

[scottstanie]

Just to record here how I made the 45 KB DEM:

1. get the frame bounding box

$ s1_info bbox tests/data/ --frame-bbox
....
tests/data/S1A_IW_SLC__1SDV_20200511T135117_20200511T135144_032518_03C421_7768.zip: [-118.4623919679569, 37.13399532964601, -115.2797133707291, 39.09331696978008]

2. Make a DEM which we'll overwrite in that area

(mapping) staniewi:s1-reader$ sardem --output-format GTiff --data cop --bbox -118.4623919679569 37.13399532964601 -115.2797133707291 39.09331696978008 -o tests/data/dummy_dem.tif

3. Use rasterio to start writing a DEM but write out no data

In [96]: src = rio.open("dummy_dem.tif")
In [99]: opts = {"SPARSE_OK": "TRUE", 'PREDICTOR': 2, 'compress': 'lzw'}
In [100]: with rio.open("dummy_dem2.tif", 'w', **opts, **src.meta) as dst:
     ...:     pass
     ...:
In [101]: src.close()

$ mv tests/data/dummy_dem2.tif  tests/data/dummy_dem.tif 
$ ls -lh tests/data/dummy_dem.tif
-rw-r--r--  1 staniewi  staff    55K Mar  6 13:12 tests/data/dummy_dem.tif

[scottstanie]

@LiangJYu I'm not sure why the test would be failing on circleCI: https://app.circleci.com/pipelines/github/opera-adt/s1-reader/337/workflows/f09e7192-4068-4d95-bfc7-5f95ceff991c/jobs/331
It says the EPSG of the DEM is -9997 ... but that's not what i'm getting locally

In [113]: import isce3
In [117]: dem_raster = isce3.io.Raster('tests/data/dummy_dem.tif')
In [118]: dem_raster.get_epsg()
Out[118]: 4326

and the tests pass for me too... so maybe someone else can checkout this PR and run pytest to see if it's just me

[vbrancat]

LGTM. Tested with COMPASS and had no issue with that. Just wondering about uploading test data in the repository. Can we have them on Zenodo? that is what we are doing with COMPASS.

[vbrancat]

I thought that our plan was not to add data to the GitHub repository but to have it be downloaded from Zenodo. Is that still the case?

[LiangJYu]

If there's no meaningful data other than EPSG in the DEM file, perhaps it can generated on the fly in the conftest as a session fixture?

[scottstanie]

yeah that sounds like a good solution too
I'm breaking off the unit test to here #108 so that Virginia can proceed with COMPASS SET testing

[vbrancat]

@scottstanie thank you for the quick fix

[LiangJYu]

@LiangJYu I'm not sure why the test would be failing on circleCI: https://app.circleci.com/pipelines/github/opera-adt/s1-reader/337/workflows/f09e7192-4068-4d95-bfc7-5f95ceff991c/jobs/331 It says the EPSG of the DEM is -9997 ... but that's not what i'm getting locally

In [113]: import isce3
In [117]: dem_raster = isce3.io.Raster('tests/data/dummy_dem.tif')
In [118]: dem_raster.get_epsg()
Out[118]: 4326

and the tests pass for me too... so maybe someone else can checkout this PR and run pytest to see if it's just me

I'm able to replicate you results above in ipython. Unable to replicate unit test fail...

[LiangJYu]

LGTM