[GRASS-dev] ​GSoC 2018 report week 03 - GRASS GIS module for Sentinel-2 cloud and shadow detection

[Moritz Lennert]

Hi Roberta !

On 03/06/18 14:38, Roberta Fagandini wrote:
> Hi all!
> I'm Roberta Fagandini and I'm working on my GSoC project, a GRASS GIS 
> module for Sentinel-2 cloud and shadow detection.
> This is my report for the third week of coding.
> 
> *1) What did I complete this week?*
> 
>   * Implemented some changes from dev feedback (e.g. r.univar instead of
>     r.stats.zonal)
>   * Tested the modified python script and fixed bugs
>   * Prepared the python script in order to start implementing the GUI
>   * Started implementing the GUI
>   * Made some changes to the code depending on the GUI requirements (add
>     controls and check on input, output and temporary file, etc.)
>   * Cleaned up the whole code
>   * Tested the GUI and fixed bugs
>   * Frequently added the basic version of the GUI to my
>     GitHub repository [0]
>   * Shared progress with the community

I'm sorry, I haven't had the time to test your module, yet. A few 
recommendation, though, based on a quick read of the code. I assume that 
the relevant file is i.sentinel.mask.

- I would recommend to immediately create a real complete module that 
can be installed with g.extension. This means creating a directory with 
the module as .py file, a .html manual page file, but which could be 
empty at the beginning, and a Makefile (see existing scripts in the core 
code or in addons for examples). Being able to install everything 
directly from github with a simple g.extension call makes testing much 
easier.

- I see that all the input bands are marked as required. Are they all 
necessary to the calculations ? If yes, aren't all these bands always 
from the same acquisition ? If yes, then maybe we can avoid parameter 
inflation by either just asking for a prefix and then getting all bands 
from that prefix (following standard sentinel naming), or provide the 
MTD file and read the band names from there, or you could ask for a 
group name and it would be up to the user to create the group before 
launching the module.

- A small idea in terms of coding style: I personally think that code 
should be as easily understandable as possible. For example: in the 
formulas you use the f_bands[] list. Maybe you could make this into a 
dictionary and so instead of reading f_bands[0], we would read 
f_bands['blue'] making the formulas easier to follow.

- Please follow PEP8 style guide (see [1]). Amongst others, this means 
limiting lines to 79 characters. So:

	- The general style used for gscript.*_command calls in Python scripts is

	gscript.run_command('v.dissolve',
			    input=tmp["centroid"],
			    column='value',
			    output=tmp["dissolve"],
			    overwrite=True,
			    quiet=True)



	i.e. each parameter on its own line. This make it easier to read
	and lines shorter.


	- The same for r.mapcalc calls: if necessary you can construct
	the individual rules over more than one line.

I'll try to find some time this week to test the module and maybe come 
back with some more feedback on the coding.

Great job so far, though !

Moritz

[1] https://trac.osgeo.org/grass/wiki/Submitting/Python#Style