Add i.saocom and i.sar toolsets

[sase1988]

The i.saocom.toolset is intended to read SAOCOM-1A L1A images. It currently comprises two basic modules:

• i.saocom.import to import SAOCOM SLC data as real and imaginary bands for the specified polarimetric channels
• i.saocom.geocode to project these images or derived products from radar to projected coordinates

The use of the modules can be tried with this sample SAOCOM dataset. It can be processed either as a zip file or as an uncompressed folder.

The i.sar toolset in inteded for general use on any SAR image. It currently comprises three modules. One already existed as a stand-alone tool (i.sar.speckle), and the other two are new.

• i.sar.speckle: This module already exists as an individual tool in GRASS addons, but other filtering options have been added.
• i.sar.amplitude: Tool intended to calculate an amplitude image from two SAR real and imaginary bands
• i.sar.pauliRGB: Tool intended to calculate the RGB bands used as input for the Pauli Composition

[wenzeslaus]

Thank you for these tools! While it is not a hard requirement at this point, it would be really good if you provide also tests for these tools. The tests are extremely useful for long-term maintenance and will soon become a crucial part of the day-to-day updates of the grass-addons repository. The tests can be written with 1) unittest-based tests which provide similar workflow experience as when writing a tool or an example or 2) pytest tests which are used experimentally now and may become more prominent later if successful.

[neteler]

Thanks for your contribution!

One thing:

i.sar.speckle: This module already exists as an individual tool in GRASS addons, but other filtering options have been added.

Should the original src/imagery/i.sar.speckle/ be removed then? Otherwise we get (partially) duplicate code. What do you think @madi ?

[veroandreo]

Gracias @sase1988 !!

I can't help it to mention that this contribution is Santiago's final work for the GRASS GIS course where students were offered different options of final work to pass the course. Proud teacher and friend here! 🤩

[sase1988]

Thanks for your contribution!

One thing:

i.sar.speckle: This module already exists as an individual tool in GRASS addons, but other filtering options have been added.

Should the original src/imagery/i.sar.speckle/ be removed then? Otherwise we get (partially) duplicate code. What do you think @madi ?

Hello Markus! My proposal is to do so, in order to have all the SAR tools inside the same toolset. Of course, after the addon is approved 😄

[wenzeslaus]

Having "all the SAR tools inside the same toolset" sounds reasonable.

While potentially complicating this PR, the clearest way might be to resolve this within one PR given that the name is the same. The issue with this in general is that moving a tool under a group breaks g.extension the.tool.name.

We don't give any promises on which addons install (although maybe we should), so formally it is okay, but still, we are trying hard to keep things working in general. With this in mind, perhaps leaving the original in place, but marking it or even replacing it with the new one (100% duplication) is on the table from my perspective.

[ninsbl]

Cool stuff. I am very much looking forward to testing the new addons.

[madi]

Thanks for your contribution!

One thing:

i.sar.speckle: This module already exists as an individual tool in GRASS addons, but other filtering options have been added.

Should the original src/imagery/i.sar.speckle/ be removed then? Otherwise we get (partially) duplicate code. What do you think @madi ?

Ciao @neteler and all. I'm glad to see more cool stuff related to SAR! It makes complete sense to me to have everything in one place, feel free to remove the addon that I created if it is a replication.

Cheers

[sase1988]

Thank you for these tools! While it is not a hard requirement at this point, it would be really good if you provide also tests for these tools. The tests are extremely useful for long-term maintenance and will soon become a crucial part of the day-to-day updates of the grass-addons repository. The tests can be written with 1) unittest-based tests which provide similar workflow experience as when writing a tool or an example or 2) pytest tests which are used experimentally now and may become more prominent later if successful.

Dear Vaclav,

Thanks a lot for your feedback. I am now working on building a test script for i.saocom.import based on the unittest structure.

• I've seen that there are many assert() methods that could be very useful. The script file I attach (I changed the extension to .txt) runs the import of a sample SAOCOM dataset and compares it to a reference raster map (assertRastersEqual). It also checks if the imported raster exists (assertRasterExists). Does it make sense using assertRastersEqual in this context?
• The script can be run under certain conditions: That GRASS has been initiated in a mapset where the reference raster exists. Does this make sense? I've seen the same logic in other codes like test_r_centroids. In case it does, how or where should I indicate this "base conditions", how should sample data be available if the file are large in terms of disk space?

I would appreciate any guidance on this, I am quite new to this stuff of creating tests and I'm very interested in learning about them ;)

Cheers

test_i_saocom_import.txt

[veroandreo]

I mostly check format stuff and language. I still find it difficult to understand how the i.saocom tools work, perhaps a general explanation plus a flow diagram could be included in the i.socom.html

[veroandreo]

Suggested change

The tool will write the output to a Geotiff external file in the directory specified in the <b>dbase</b> option. The output files will be projected to the CS of the target location indicated in the option <b>location</b>. If the specified location does not exits, GRASS will create it with the CS EPSG:4326. The output map name will we the same as the input map, with the suffix <em>_geo</em> added at the end.

The tool will write the output to a Geotiff external file in the directory specified by the <b>dbase</b> option. The output files will be projected to the CS of the target location indicated in the option <b>location</b>. If the specified location does not exist, GRASS will create it with the CS EPSG:4326. The output map name will be the same as the input map, with the suffix <em>_geo</em> added at the end.

[veroandreo]

Why do you need to specify a target location to get the geocoded data? The module will always be run from a certain location, hence that should be the target location, i.e., I'm running i.saocom.geocode from a UTM location, that's where I want the output to be, right? Hence no need for such a parameter.

[petrasovaa]

As a better practice, please move this into at the beginning of main function.

[sase1988]

What should I exactly move? the line options, flags = gscript.parser()?

[petrasovaa]

Yes, exactly

[petrasovaa]

Move this line into main()

[sase1988]

Same doubt as #949 (comment)

[petrasovaa]

Please see best practices for creating temporary maps:
https://github.com/wenzeslaus/foss4g-2022-developing-custom-grass-tools/blob/main/notebook_3_best_practices.ipynb (look for Temporary Maps section)
(This will eventually end up in official documentation...)

The idea is to create safe names, register them so that they get removed if something fails, and avoid accidental removal of maps created by different processes.

[sase1988]

Thank you for this comment. I have changed it trying to follow the recommended practices, but just in case I commented the original code lines.

[sase1988]

@sase1988 Looking at the saocom tools, could you explain what are they actually doing, I am not familiar with SAR data much. My specific issue is that you are using a lot external dependencies. Some of them seem unused. I would like to avoid as many as possible. I would prefer using osgeo package (python gdal bindings) rather than rasterio since osgeo is already used in other GRASS tools and I would think it would have all the functionality you need (maybe not?). But in general, I feel like the entire workflow of exporting and importing and creating new location could be improved from the point of integration with GRASS, but it would be helpful to have little more explanation.

Dear Anna, thank you very much for you feedback.

Regarding the detail information, where or how (which format) should I give a more in-depth explanation? As suggested by @veroandreo I am drawing some graphical workflows to add to the .html files, but the detail about the external dependencies and how they are used in each step, and why I prefer not using more low level bindings such as GDAL, might be too much for the tool final user's document.

[petrasovaa]

@sase1988 Looking at the saocom tools, could you explain what are they actually doing, I am not familiar with SAR data much. My specific issue is that you are using a lot external dependencies. Some of them seem unused. I would like to avoid as many as possible. I would prefer using osgeo package (python gdal bindings) rather than rasterio since osgeo is already used in other GRASS tools and I would think it would have all the functionality you need (maybe not?). But in general, I feel like the entire workflow of exporting and importing and creating new location could be improved from the point of integration with GRASS, but it would be helpful to have little more explanation.

Dear Anna, thank you very much for you feedback.

Regarding the detail information, where or how (which format) should I give a more in-depth explanation? As suggested by @veroandreo I am drawing some graphical workflows to add to the .html files, but the detail about the external dependencies and how they are used in each step, and why I prefer not using more low level bindings such as GDAL, might be too much for the tool final user's document.

Right, I think discussing it right here in the PR is appropriate. I still need to look at it in more detail, but it looks like you import GDAL package, but don't use it and instead call gdalwarp tool. Any reason why you use affine instead of rasterio.affine? In any case, the dependencies should be communicated in the manual and there should be lazy loading otherwise the addons won't be built.

[sase1988]

@sase1988 Looking at the saocom tools, could you explain what are they actually doing, I am not familiar with SAR data much. My specific issue is that you are using a lot external dependencies. Some of them seem unused. I would like to avoid as many as possible. I would prefer using osgeo package (python gdal bindings) rather than rasterio since osgeo is already used in other GRASS tools and I would think it would have all the functionality you need (maybe not?). But in general, I feel like the entire workflow of exporting and importing and creating new location could be improved from the point of integration with GRASS, but it would be helpful to have little more explanation.

Dear Anna, thank you very much for you feedback.
Regarding the detail information, where or how (which format) should I give a more in-depth explanation? As suggested by @veroandreo I am drawing some graphical workflows to add to the .html files, but the detail about the external dependencies and how they are used in each step, and why I prefer not using more low level bindings such as GDAL, might be too much for the tool final user's document.

Right, I think discussing it right here in the PR is appropriate. I still need to look at it in more detail, but it looks like you import GDAL package, but don't use it and instead call gdalwarp tool. Any reason why you use affine instead of rasterio.affine? In any case, the dependencies should be communicated in the manual and there should be lazy loading otherwise the addons won't be built.

Dear Anna,
I am working on replacing the use of rasterio with GDAL. It makes the code a little bit longer, but in the end it will use GDAL only. The main complication is to find a GDAL simple method to do the same as rasterio.transform.from_gcps(). I have also eliminated many libraries that indeed were unused.

[petrasovaa]

Dear Anna, I am working on replacing the use of rasterio with GDAL. It makes the code a little bit longer, but in the end it will use GDAL only. The main complication is to find a GDAL simple method to do the same as rasterio.transform.from_gcps(). I have also eliminated many libraries that indeed were unused.

I don't insist on replacing rasterio if that's a problem... I was trying to understand which dependencies are necessary and which ones were used e.g. just because of a better syntax. Generally fewer extra dependencies makes it simpler for the user.

[petrasovaa]

Looking at the diagrams, I was trying to understand the workflow and this is still unclear, I may be missing something:
Why are these 2 modules and not one, just the geocode? What is the purpose of importing it into a temporary XY location and then exporting tif from there, doing the geocoding via external tools and then importing it again, it seems the temporary XY location step is not doing anything?

Also, as Vero noted earlier, the geocode tool should import it to the current location, not create a new one and import it there. Is there any reason it needs to be epsg 4326, it seems at that point any CRS should be acceptable, no?

[petrasovaa]

This is a lot of unnecessary code duplication. I would suggest using zipfile.is_zipfile to detect if it's a zip file automatically (and remove the associated module option) and then merging these 2 functions.