isce-framework / s1-reader

Sentinel-1 reader
Apache License 2.0
27 stars 12 forks source link

annotation reader (s1_annotation.py) - initial implementation #48

Closed seongsujeong closed 2 years ago

seongsujeong commented 2 years ago

This PR implements the reader for Calibration Annotation Data Set (CADS), and Noise Annotation Data Set (NADS), which was related to the issue #27. This work is in context of implementing thermal noise correction. The structure of CADS and NADS are very similar, so their readers shares similar procedure to read. Therefore the readers are implemented together in s1_annotation.py

Few examples of the usage are as below:

import s1_annotation
cads=calibration('/Users/USER_NAME/DATA/COMPASS/S1B_IW_SLC__1SDV_20210112T014942_20210112T015009_025115_02FD69_DDC9.zip',polarization='VV',subswath=2)
nads=noise('/Users/USER_NAME/DATA/COMPASS/SAFE/S1B_IW_SLC__1SDV_20210112T014942_20210112T015009_025115_02FD69_DDC9',polarization='VV',subswath=1) #can assign a SAFE directory

cads.IPF_version #'003.31' for example. Also works for nads
cads.list_gamma

nads.rg_list_NoiseRangeLut #Range noise vector
nads.az_line #Range noise vector
nads.az_noiseAzimuthLut #Azinuth noise vector

Notes:

seongsujeong commented 2 years ago

Thank you all for valuable feedback. I revised the code as much as I could based on the comments posted here. One last thing I would like to discuss is what data type would be the best for Sentinel1BurstSlc.ipf_version. Please see the the comments below for the detail.

https://github.com/opera-adt/s1-reader/pull/48#discussion_r927925747

I am inclined to packaging.version.Version. for the version information (and accept the suggestion by @LiangJYu ), but any comments and suggestion are appreciated.

seongsujeong commented 2 years ago

Thanks @LiangJYu @gshiroma for the wonderful suggestions and information to improve the code. I think the code is ready to review.

seongsujeong commented 2 years ago

Thank you all for the inputs for this PR. I've updated the branch to date to test it before merging it into the main repo. Looks like pytest in CircieCI is failing, with the error summary below:

=============================================== short test summary info =============================================== ERROR tests/test_bursts.py::test_burst - KeyError: "There is no item named 'manifest.safe' in the archive" ERROR tests/test_orbit.py::test_orbit_datetime - KeyError: "There is no item named 'manifest.safe' in the archive" ERROR tests/test_reader.py::test_burst_from_zip - KeyError: "There is no item named 'manifest.safe' in the archive"

I've taken a look at the test data .../tests/data/S1A_IW_SLC__1SDV_20200511T135117_20200511T135144_032518_03C421_7768.zip, and found that the file manifest.safe is missing in the .zip file. This PR includes updates on s1_reader.py that reads IPF version from the manifest.safe file. I wonder if someone can commit a new .zip file for the test data that has the file.

gshiroma commented 2 years ago

I also forgot to ask you to please resolve the codacy issues.

Nevermind. I just noticed you did that yesterday. Thanks!

LiangJYu commented 2 years ago

Thank you all for the inputs for this PR. I've updated the branch to date to test it before merging it into the main repo. Looks like pytest in CircieCI is failing, with the error summary below:

=============================================== short test summary info =============================================== ERROR tests/test_bursts.py::test_burst - KeyError: "There is no item named 'manifest.safe' in the archive" ERROR tests/test_orbit.py::test_orbit_datetime - KeyError: "There is no item named 'manifest.safe' in the archive" ERROR tests/test_reader.py::test_burst_from_zip - KeyError: "There is no item named 'manifest.safe' in the archive"

I've taken a look at the test data .../tests/data/S1A_IW_SLC__1SDV_20200511T135117_20200511T135144_032518_03C421_7768.zip, and found that the file manifest.safe is missing in the .zip file. This PR includes updates on s1_reader.py that reads IPF version from the manifest.safe file. I wonder if someone can commit a new .zip file for the test data that has the file.

I'm working this and will submit a PR to this branch sometime today.

LiangJYu commented 2 years ago

The scope of the fix grew being adding manifest.safe so I issued a new PR to main instead of to your PR. See #58

seongsujeong commented 2 years ago

The scope of the fix grew being adding manifest.safe so I issued a new PR to main instead of to your PR. See #58

Also left a commend on the PR #58, but confirmed that CircleCI passed. Thank you for the update on the unit test!