<div dir="ltr">Hi all!<div><br></div><div>Thank you, Moritz and Nikos, for your suggestions!<br><div class="gmail_extra"><br><div class="gmail_quote">2018-06-04 11:54 GMT+02:00 Nikos Alexandris <span dir="ltr"><<a href="mailto:nik@nikosalexandris.net" target="_blank">nik@nikosalexandris.net</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Ciao Roberta,<br>
<br>
great work!<br>
<br>
Moritz got me faster, as I intended to chime in, with similar<br>
suggestions.<br>
<br>
So +1 for:<br>
<br>
- a "real" module, as Moritz described it<br></blockquote><div><br></div><div>Done! I added the folder to my github repository but I have some trouble compiling the module with g.extension:</div><div><br></div><div>g.extension extension=i.sentinel.mask operation=add url=<a href="https://github.com/RobiFag/GRASS_clouds_and_shadows">https://github.com/RobiFag/GRASS_clouds_and_shadows</a><br></div><div><br></div><div>error message:</div><div><div>g.extension extension=i.sentinel.mask operation=add url=<a href="https://github.com/RobiFag/GRASS_clouds_and_shadows">https://github.com/RobiFag/GRASS_clouds_and_shadows</a></div><div>Fetching <i.sentinel.mask> from <<a href="https://github.com/RobiFag/GRASS_clouds_and_shadows/archive/master.zip">https://github.com/RobiFag/GRASS_clouds_and_shadows/archive/master.zip</a>> (be patient)...</div><div>Compiling...</div><div>make[1]: *** No rule to make target `/tmp/grass7-roberta-701</div><div>4/tmpe7QEso/i.sentinel.mask/scripts/i.sentinel.mask', needed</div><div>by `script'. Stop.</div><div>/bin/sh: 1: cannot create /usr/lib/grass74/error.log:</div><div>Permission denied</div><div>make: *** [i.sentinel.mask] Error 2</div><div>ERROR: Compilation failed, sorry. Please check above error messages.</div></div><div><br></div><div>any suggestion?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
- for using the metadata file, and parsing it -- my experience with<br>
 Landsat and other high-resolution satellite products, showed it's best to<br>
 trust the individual metadata that accompany an acquisition.<br>
  Parameters may be very specific to one single acquisition.<br>
 Also, a module that is designed to be "dynamic" is more<br>
 future-proof and easy to maintain and update.<br>
  For example, reading band names from the meta-data, as opposed to more<br>
 "static" designs, i.e. expecting specific hardcoded options, such as<br>
 band names, or their order.<br></blockquote><div> </div><div><div>I have already thought about parsing the metadatafile to retrieve the input bands but at the moment the module requires atmospherically corrected bands and if users perform the atmospheric correction on their own I don't have any control on naming. Moreover, the module needs to know which raster map corresponds to the blue band, to the green band and so on in order to apply the rules for clouds and shadows detection. Therefore I need users specify each band but maybe I can manage this issue requiring a specific suffix in the bands' names (e.g. blabla_blue, blabla_green, etc.). In this way, users have to provide only the prefix (blabla) while the module search for all raster maps with the specified prefix and at the same time it is able to recognize each band.</div><div><br></div><div>Anyway, the aim of the next coding period is to create a python script that wraps in a single GRASS module the download and import phase (i.sentinel.download and i.sentinel.import), the atmospheric correction using i.atcorr and the cloud and shadow detection procedure. In particular, for what concerns the atmospheric correction, I'd like to implement an iterative procedure that executes i.atcorr for all bands of the input image changing accordingly the requested input parameters. This part should simplify the management of input maps.</div></div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
- PEP8 rules and keeping module options in separate lines to increase legibility<br>
<br>
In line with the suggestions on style, I would stress out to fully<br>
spell out the names of functions, options, descriptions of flags,<br>
iterables and iterators.  It will make it easier for the next reader of<br>
your code.<br></blockquote><div><br></div><div>Yes, you are both right!! I have to clean up the code from the style point of view and it will be one of the tasks of this week and especially of the last days before the GSoC Evaluation.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Finally, it's a good idea to write even mini-tests, while you are<br>
progressing, for the small functions that eventually develop to keep the<br>
full scrip compact, functional and more legible.<br>
<br>
Writing a test, for your module, in the end, will be much easier.<br></blockquote><div><br></div><div>Sorry Nikos but I don't really understand what you mean by "mini-tests"..are you referring to the examples in the manual page?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
I am sure you know all this. Yet, discussing it is a good thing in<br>
my view.<br></blockquote><div><br></div><div>Discussing it is very useful especially for me so please do not take anything for granted and keep sending me your feedback!</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Keep up the good work, Nikos<br></blockquote><div><br></div><div>Cheers!</div><div>Roberta</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
<br>
* Moritz Lennert <<a href="mailto:mlennert@club.worldonline.be" target="_blank">mlennert@club.worldonline.be</a>> [2018-06-04 11:03:41 +0200]:<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail-m_-1872138991244865m_-3511696702576688460m_4953242729304083505gmail-m_2093213739163109453m_4041964427931249679h5">
Hi Roberta !<br>
<br>
On 03/06/18 14:38, Roberta Fagandini wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Hi all!<br>
I'm Roberta Fagandini and I'm working on my GSoC project, a GRASS GIS module for Sentinel-2 cloud and shadow detection.<br>
This is my report for the third week of coding.<br>
<br>
*1) What did I complete this week?*<br>
<br>
 * Implemented some changes from dev feedback (e.g. r.univar instead of<br>
   r.stats.zonal)<br>
 * Tested the modified python script and fixed bugs<br>
 * Prepared the python script in order to start implementing the GUI<br>
 * Started implementing the GUI<br>
 * Made some changes to the code depending on the GUI requirements (add<br>
   controls and check on input, output and temporary file, etc.)<br>
 * Cleaned up the whole code<br>
 * Tested the GUI and fixed bugs<br>
 * Frequently added the basic version of the GUI to my<br>
   GitHub repository [0]<br>
 * Shared progress with the community<br>
</blockquote>
<br>
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.<br>
<br>
- 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.<br>
<br>
- 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.<br>
<br>
- 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.<br>
<br>
- Please follow PEP8 style guide (see [1]). Amongst others, this means limiting lines to 79 characters. So:<br>
<br>
        - The general style used for gscript.*_command calls in Python scripts is<br>
<br>
        gscript.run_command('v.dissolv<wbr>e',<br>
                            input=tmp["centroid"],<br>
                            column='value',<br>
                            output=tmp["dissolve"],<br>
                            overwrite=True,<br>
                            quiet=True)<br>
<br>
<br>
<br>
        i.e. each parameter on its own line. This make it easier to read<br>
        and lines shorter.<br>
<br>
<br>
        - The same for r.mapcalc calls: if necessary you can construct<br>
        the individual rules over more than one line.<br>
<br>
I'll try to find some time this week to test the module and maybe come back with some more feedback on the coding.<br>
<br>
Great job so far, though !<br>
<br>
Moritz<br>
<br>
[1] <a href="https://trac.osgeo.org/grass/wiki/Submitting/Python#Style" rel="noreferrer" target="_blank">https://trac.osgeo.org/grass/w<wbr>iki/Submitting/Python#Style</a><br>
<br>
<br></div></div>
______________________________<wbr>_________________<br>
grass-dev mailing list<br>
<a href="mailto:grass-dev@lists.osgeo.org" target="_blank">grass-dev@lists.osgeo.org</a><br>
<a href="https://lists.osgeo.org/mailman/listinfo/grass-dev" rel="noreferrer" target="_blank">https://lists.osgeo.org/mailma<wbr>n/listinfo/grass-dev</a><span class="gmail-m_-1872138991244865m_-3511696702576688460m_4953242729304083505gmail-m_2093213739163109453m_4041964427931249679HOEnZb"><font color="#888888"><br>
</font></span></blockquote><span class="gmail-m_-1872138991244865m_-3511696702576688460m_4953242729304083505gmail-m_2093213739163109453m_4041964427931249679HOEnZb"><font color="#888888">
<br>
-- <br>
Nikos Alexandris | Remote Sensing & Geomatics<br>
GPG Key Fingerprint 6F9D4506F3CA28380974D31A905353<wbr>4B693C4FB3 </font></span></blockquote></div><br></div></div></div>