GeoscienceAustralia / PyRate

A Python tool for estimating velocity and time-series from Interferometric Synthetic Aperture Radar (InSAR) data.
https://geoscienceaustralia.github.io/PyRate/
Apache License 2.0
200 stars 70 forks source link

added to geographic_coordinate to pixel and pixel to geographic_coord… #292

Closed basaks closed 4 years ago

basaks commented 4 years ago

Added to refpixel geographic_coordinate to pixel tests Added to refpixel pixel to geographic_coordinate tests

codecov-commenter commented 4 years ago

Codecov Report

Merging #292 into sb/step4-refactor-process will decrease coverage by 1.47%. The diff coverage is 100.00%.

Impacted file tree graph

@@                      Coverage Diff                      @@
##           sb/step4-refactor-process     #292      +/-   ##
=============================================================
- Coverage                      82.73%   81.26%   -1.48%     
=============================================================
  Files                             26       26              
  Lines                           3499     3592      +93     
  Branches                         549      578      +29     
=============================================================
+ Hits                            2895     2919      +24     
- Misses                           505      567      +62     
- Partials                          99      106       +7     
Impacted Files Coverage Δ
pyrate/core/refpixel.py 93.11% <100.00%> (ø)
pyrate/constants.py 100.00% <0.00%> (ø)
pyrate/merge.py 15.60% <0.00%> (+0.12%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d0dd46e...b213b24. Read the comment docs.

tfuhrmann commented 4 years ago

I've checked the change in the code of test_refpixel.py and checked the results using 'pytest'. Works as expected. I'm wondering though if the +1/-1 buffer is needed for the test data sets. In my test runs the gdallocationinfo result and the result of the convert_pixel_value_to_geographic_coordinate function were always identical for the 25 sets of test coordinates, e.g.: gdal output:

  Location: (13P,36L)
  Band 1:
    Value: 284

convert_pixel_value_to_geographic_coordinate function result: 13 36

I remember a discussion with Sheece on this where we found differences of one pixel in the two calculation methods. But it might be that this was related to legacy code.

Alright, I've tried the code without the +/-1 and re-run the test 100 times. Turns out that there is some locations where the +/-1 is needed, e.g.:

>           assert f"({x}P,{y}L" in out
E           AssertionError: assert '(27P,6L' in 'Report:\n  Location: (26P,6L)\n  Band 1:\n    Value: 290\n'

So, all good with the current implementation!