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

seongsujeong · 2022-06-17T04:19:25Z

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:

in s1_annotation.noise, the attribues related to range noise vector has prefix rg, and the ones about the azimuth noise vector has az
The information on the annotation are parsed on-the-fly.

gshiroma

Thank you for adding this, @seongsujeong . That's nice that you wrote a class that can be used for both radiometric calibration and thermal noise correction. This is a good start. I have a few comments/suggestions below. I'd also suggest running this code through some PEP8 checker. I'm looking forward to having this class used by the S-1 reader.

src/s1reader/s1_annotation.py

LiangJYu

Apologies in advance - My comments are effectively asking for a rewrite of this.

Not in the PR but should be is the inclusion and integration of Calibration and Noise with the rest s1reader modules. I think Calibration and Noise should be attributes of the Sentinel1BurstSlc dataclass. The init of these objects should occur when a Sentinel1BurstSlc is initalized.

Let me know if you have any questions.

src/s1reader/s1_annotation.py

hfattahi

@seongsujeong thank you for this nice PR. As we briefly discussed offline, I think it will be great to slightly restructure this PR such that the Noise and Calibration are returned as isce3.core.LUT2D objects which are class members of the existing Sentinel1BurstSlc class. Since they are small and trivial to read, I think they should be read by default as we read the data and should not require an extra call. At a higher level workflow a user of the reader should not do any extra work and the returned burst should have LUT2D of noise and calibration. Please let me know if you have any concern or if this my comment is not clear.

…stSlc instances

src/s1reader/s1_annotation.py

LiangJYu

Apologies for this incomplete review. I'll try to finish reviewing sometime this coming week.

src/s1reader/s1_reader.py

src/s1reader/s1_annotation.py

LiangJYu

@seongsujeong nice work! Few comments before it's ready.

src/s1reader/s1_annotation.py

src/s1reader/s1_reader.py

src/s1reader/s1_annotation.py

Co-authored-by: Liang Yu <[email protected]>

… into annotation_reader

Co-authored-by: Liang Yu <[email protected]>

… into annotation_reader

Co-authored-by: Liang Yu <[email protected]>

Updating branch `annotation_reader` in this for to date

gshiroma

Nice work, @seongsujeong . Thank you for working on that. I still have some minor comments before we merge this PR.

src/s1reader/s1_annotation.py

src/s1reader/s1_reader.py

seongsujeong · 2022-08-02T06:44:05Z

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

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

gshiroma · 2022-08-02T16:50:02Z

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

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

LiangJYu · 2022-08-02T18:37:25Z

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 · 2022-08-02T19:38:57Z

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

Unit test fixes from isce-framework#58

seongsujeong · 2022-08-02T22:33:54Z

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!

Co-authored-by: Gustavo H. X. Shiroma <[email protected]>

… into annotation_reader

gshiroma

LGTM. Thank you, @seongsujeong , for addressing all my comments. Just please make sure to address all new Codacy issues before merging it.

@gshiroma

* annotation reader (s1_annotation.py) init. commit * s1_annotation.py - rev1 after the suggestion by @gshiroma * s1_annotation.py - Pylinted to be more conform to PEP8 * annotation objects are attributes of `Sentinel1BurstSlc` * s1_annotation.py : Classes for Burst-wide information * s1_reader.py - annotation information are stored in each Sentinel1BurstSlc instances * fix on `s1_annotation.parse_polynomial_element()` to read `azimuthFmRatePolynomial` in older format. * Adding features for applying beta0 * s1_reader.py - differentiating the polynomial parsing scheme based on exception handling * thermal noise LUT for IPF lower than 2.90 * Update src/s1reader/s1_reader.py - comment from Liang Co-authored-by: Liang Yu <[email protected]> * Fix on edits committed on another branch * removing placeholder Co-authored-by: Liang Yu <[email protected]> * get_ipf_version - revision by @LiangJYu Co-authored-by: Liang Yu <[email protected]> * Update src/s1reader/s1_reader.py - remove placeholder Co-authored-by: Liang Yu <[email protected]> * moving the single-use variables closer to where they are used. Co-authored-by: Liang Yu <[email protected]> * Update src/s1reader/s1_reader.py Co-authored-by: Liang Yu <[email protected]> * s1_annotation.py - readibility improvement * Update on docstring for `AnnotationBase` Co-authored-by: Liang Yu <[email protected]> * revision on error message Co-authored-by: Liang Yu <[email protected]> * Addressing comments by @LiangJYu and @vbrancat in PR #48 * s1_annotation.py - code cleanup Co-authored-by: Liang Yu <[email protected]> * s1_annotation.py - readability improvement Co-authored-by: Liang Yu <[email protected]> * readibility improvement * changing the datatype of IPF version to version.Version. Changing the affected functions accordingly. * space between args in _parse_list * addressing the comments by @vbrancat * added manifest.safe to test SAFE/zip * updated burst ID format * fixed directory structure in zip to match S1 SAFE spec * Update src/s1reader/s1_annotation.py Co-authored-by: Gustavo H. X. Shiroma <[email protected]> * Update src/s1reader/s1_annotation.py Co-authored-by: Gustavo H. X. Shiroma <[email protected]> * Update src/s1reader/s1_annotation.py Co-authored-by: Gustavo H. X. Shiroma <[email protected]> * Update src/s1reader/s1_annotation.py Co-authored-by: Gustavo H. X. Shiroma <[email protected]> * s1_annotation.py - fixing the logic to add margin to azimuth noise LUT Co-authored-by: Gustavo H. X. Shiroma <[email protected]> * s1_reader.py - removing lines that commented out Co-authored-by: Gustavo H. X. Shiroma <[email protected]> * s1_reader.py - further removal of lines that commented out Co-authored-by: Gustavo H. X. Shiroma <[email protected]> * Addressing comments by @gshiroma and other minor fix * Addressing codacy issues * Taking care of codacy issues * Fixing Codacy issue * Fix on taking care of some old IPF version when loading AZ FM rate polynomial * fix on readability * fix on PEP issue * Addressing the comment by @gshiroma * more context and description to the AZ FM rate format change Co-authored-by: Seongsu Jeong <[email protected]> Co-authored-by: Liang Yu <[email protected]> Co-authored-by: Gustavo H. X. Shiroma <[email protected]>

annotation reader (s1_annotation.py) init. commit

df94aec

seongsujeong requested review from hfattahi, gshiroma and vbrancat June 17, 2022 04:19

hfattahi requested review from LiangJYu and yunjunz June 17, 2022 06:44

gshiroma reviewed Jun 21, 2022

View reviewed changes

Seongsu Jeong added 2 commits June 22, 2022 15:12

s1_annotation.py - rev1 after the suggestion by @gshiroma

14569b1

s1_annotation.py - Pylinted to be more conform to PEP8

7e85dd7

LiangJYu reviewed Jun 24, 2022

View reviewed changes

hfattahi reviewed Jun 28, 2022

View reviewed changes

Seongsu Jeong added 3 commits July 8, 2022 14:19

annotation objects are attributes of Sentinel1BurstSlc

d102ac1

s1_annotation.py : Classes for Burst-wide information

aa11b0b

s1_reader.py - annotation information are stored in each Sentinel1Bur…

e47d5f4

…stSlc instances

vbrancat reviewed Jul 16, 2022

View reviewed changes

src/s1reader/s1_annotation.py Outdated Show resolved Hide resolved

LiangJYu reviewed Jul 17, 2022

View reviewed changes

LiangJYu reviewed Jul 21, 2022

View reviewed changes

seongsujeong and others added 13 commits July 22, 2022 09:29

Update src/s1reader/s1_reader.py - comment from Liang

5d069d8

Co-authored-by: Liang Yu <[email protected]>

Fix on edits committed on another branch

dbbfb0c

Merge branch 'annotation_reader' of github.com:seongsujeong/s1-reader…

e6a0db3

… into annotation_reader

removing placeholder

5a948d9

Co-authored-by: Liang Yu <[email protected]>

get_ipf_version - revision by @LiangJYu

22a1aa2

Co-authored-by: Liang Yu <[email protected]>

Update src/s1reader/s1_reader.py - remove placeholder

5251d9b

Co-authored-by: Liang Yu <[email protected]>

moving the single-use variables closer to where they are used.

ffbdfa0

Co-authored-by: Liang Yu <[email protected]>

Update src/s1reader/s1_reader.py

52fbfe4

Co-authored-by: Liang Yu <[email protected]>

Merge branch 'annotation_reader' of github.com:seongsujeong/s1-reader…

29dea18

… into annotation_reader

s1_annotation.py - readibility improvement

c69ba60

Update on docstring for AnnotationBase

cf6ec23

Co-authored-by: Liang Yu <[email protected]>

revision on error message

729bc4a

Co-authored-by: Liang Yu <[email protected]>

Addressing comments by @LiangJYu and @vbrancat in PR isce-framework#48

7fa184f

Seongsu Jeong and others added 2 commits August 1, 2022 22:35

addressing the comments by @vbrancat

1ca4f68

Merge pull request #1 from opera-adt/main

2986152

Updating branch `annotation_reader` in this for to date

gshiroma reviewed Aug 2, 2022

View reviewed changes

seongsujeong mentioned this pull request Aug 2, 2022

unit tests #38

Merged

gshiroma reviewed Aug 2, 2022

View reviewed changes

LiangJYu added 3 commits August 2, 2022 12:33

added manifest.safe to test SAFE/zip

2d7aab2

updated burst ID format

3cbafde

fixed directory structure in zip to match S1 SAFE spec

2b4b8e7

Merge pull request #2 from LiangJYu/unit_test_fixes

092b4d4

Unit test fixes from isce-framework#58

seongsujeong mentioned this pull request Aug 2, 2022

Unit test fixes #58

Merged

seongsujeong and others added 9 commits August 2, 2022 15:54

Update src/s1reader/s1_annotation.py

caf4751

Co-authored-by: Gustavo H. X. Shiroma <[email protected]>

Update src/s1reader/s1_annotation.py

63cb63d

Co-authored-by: Gustavo H. X. Shiroma <[email protected]>

Update src/s1reader/s1_annotation.py

b88262d

Co-authored-by: Gustavo H. X. Shiroma <[email protected]>

Update src/s1reader/s1_annotation.py

b373bda

Co-authored-by: Gustavo H. X. Shiroma <[email protected]>

s1_annotation.py - fixing the logic to add margin to azimuth noise LUT

a5c15e8

Co-authored-by: Gustavo H. X. Shiroma <[email protected]>

s1_reader.py - removing lines that commented out

23c4f1e

Co-authored-by: Gustavo H. X. Shiroma <[email protected]>

s1_reader.py - further removal of lines that commented out

0dc297b

Co-authored-by: Gustavo H. X. Shiroma <[email protected]>

Addressing comments by @gshiroma and other minor fix

0dccf18

Merge branch 'annotation_reader' of github.com:seongsujeong/s1-reader…

14af881

… into annotation_reader

gshiroma approved these changes Aug 6, 2022

View reviewed changes

Seongsu Jeong added 3 commits August 7, 2022 20:47

Addressing codacy issues

dba63fa

Taking care of codacy issues

f09407f

Fixing Codacy issue

2295178

seongsujeong merged commit cfb0443 into isce-framework:main Aug 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

seongsujeong commented Jun 17, 2022

gshiroma left a comment

LiangJYu left a comment

hfattahi left a comment

LiangJYu left a comment

LiangJYu left a comment

gshiroma left a comment

seongsujeong commented Aug 2, 2022

gshiroma left a comment

gshiroma commented Aug 2, 2022 •

edited

Loading

LiangJYu commented Aug 2, 2022

LiangJYu commented Aug 2, 2022

seongsujeong commented Aug 2, 2022

gshiroma left a comment

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

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

Conversation

seongsujeong commented Jun 17, 2022

gshiroma left a comment

Choose a reason for hiding this comment

LiangJYu left a comment

Choose a reason for hiding this comment

hfattahi left a comment

Choose a reason for hiding this comment

LiangJYu left a comment

Choose a reason for hiding this comment

LiangJYu left a comment

Choose a reason for hiding this comment

gshiroma left a comment

Choose a reason for hiding this comment

seongsujeong commented Aug 2, 2022

gshiroma left a comment

Choose a reason for hiding this comment

gshiroma commented Aug 2, 2022 • edited Loading

LiangJYu commented Aug 2, 2022

LiangJYu commented Aug 2, 2022

seongsujeong commented Aug 2, 2022

gshiroma left a comment

Choose a reason for hiding this comment

gshiroma commented Aug 2, 2022 •

edited

Loading