<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2018-06-11 10:29 GMT+02:00 Moritz Lennert <span dir="ltr"><<a href="mailto:mlennert@club.worldonline.be" target="_blank">mlennert@club.worldonline.be</a>></span><wbr>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Roberta,<br>
<br>
Thanks for your report and your great work !<br></blockquote><div><br></div><div>Thank to you for your support!<br></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>
Here are a few remarks:<br>
<br>
- You have a -t flag with description "Do not delete temporary files".<br>
Is this only for debugging purposes (and which temporary files are<br>
created), or does this have any real use for the user ? If the<br>
former, this should probably not be part of the module UI, if the<br>
latter, it should be explained in the man page.<br></blockquote><div><br></div><div><div>I have not yet decided whether to keep the option. At the moment it is certainly useful for debugging purposes but I also think it could be useful to users interested in understanding how the procedure works. </div><div>What do you think? Could it be useful?</div></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 have several comments concerning your input_file parameter<br>
<br>
<br>
- The syntax of this file has to be explained in the man page,<br>
including the precise variable names (blue, not Blue, not<br>
BLUE, etc)<br></blockquote><div><br></div><div>Of course! I have already added the explanation this morning.</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>
- The code for reading this file can probably be simplified<br>
quite easily by using something like this:<br>
<br>
for line in file(input_file):<br>
a = line.split('=')<br>
if len(a) <> 2 or a[0] not in ['blue', 'red', etc ]:<br>
gscript.fatal("Syntax error in the txt file.")<br>
a[1] = a[1].strip()<br>
bands[a[0]] = a[1]<br></blockquote><div><br></div><div>Thank you! this is a better solution than mine. It was a first experiment but I have already implemented your suggestion.<br></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>
- Again a (minor) remark on readability of the code. When you use named<br>
variables in format(), I would suggest that you use the actual names.<br>
i.e. instead of<br>
<br>
gscript.mapcalc('{r} = 1.0 * ({a})/{c}'.format(<br>
r=("{}_{}".format(b, d)),<br>
a=b, c=scale_fac),<br>
overwrite=True)<br>
<br>
why not use this:<br>
<br>
gscript.mapcalc('{r} = 1.0 * ({b})/{scale_fac}'.format(<br>
r=("{}_{}".format(b, d)),<br>
b=b, scale_fac=scale_fac),<br>
overwrite=True)<br>
<br>
?<br>
<br>
I think it makes the formula more easily readable.<br></blockquote><div><br></div><div>Yes, you are right! Done!<br></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>
- Some remarks on your handling of temporary maps:<br>
<br>
- You use<br>
<br>
processid = "{:.2f}".format(time.time())<br>
processid = processid.replace(".", "_")<br>
<br>
why not simply:<br>
<br>
processid = os.getpid() ?<br></blockquote><div><br></div><div>I didn't know this function, it seems to be cleaner than mine..thank you, added!<br></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>
- Instead of having to check for the type of every single map<br>
in the cleanup function, you could include the type in the<br>
info, i.e. something like this:<br>
<br>
r = 'raster'<br>
v = 'vector'<br>
<br>
tmp["cloud_v"] = ["cloud_v_" + processid, v]<br>
(I personally use "cloud_v_%i" % processid; not sure<br>
what is preferable)<br>
<br>
which then allows you to do something like this in cleanup():<br>
<br>
for temp_map, maptype in tmp:<br>
if gscript.find_file(temp_map, element=maptype)['name']:<br>
gscript.run_command('g.remove'<wbr>,<br>
flags='f',<br>
type=maptype,<br>
name=temp_map,<br>
quiet=True) <br></blockquote><div><br></div><div>I tried to implement this solution but in this way, I get a warning message:</div><div><br></div><div> WARNING: Illegal filename <cloud_v_8048,vector> etc.</div><div><br></div><div>I tried different ways and I looked for something similar but I can not find how to add the map <span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">type<span> </span></span>in the info.<br></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>
- In its final version your man page should contain an example,<br>
ideally a complete example starting with i.sentinel.download and<br>
going all the way to i.sentinal.mask in order to show the use of the<br>
module within a reproducible workflow, best using a North Carolina<br>
example to fit with the demo dataset.<br></blockquote><div><br></div><div>I had already thought about making a complete example. I hope to prepare and add this first part by the end of this week.<br></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>
Moritz<br></blockquote><div><br></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>
Le Sun, 10 Jun 2018 10:58:37 +0200,<br>
Roberta Fagandini <<a href="mailto:robifagandini@gmail.com" target="_blank">robifagandini@gmail.com</a>> a écrit :<br>
<br>
> Hi all!<br>
> I'm Roberta Fagandini and I'm working on my GSoC project, a GRASS GIS<br>
> module for Sentinel-2 cloud and shadow detection.<br>
> This is my report for the fourth week of coding.<br>
> <br>
> *1) What did I complete this week?*<br>
> <br>
> - Implemented some changes from dev feedback (e.g. added a new<br>
> flag to manage the two procedure separately and added the text file<br>
> option to specify input bands)<br>
> - Tested the modified python script and fixed bugs (e.g. solved a<br>
> bug for -s flag)<br>
> - Created a real complete GRASS GIS module that can be installed<br>
> with g.extension<br>
> - Finished implementing the GUI<br>
> - Tested the GUI and fixed bugs<br>
> - Cleaned up the code from the style point of view in order to<br>
> make it more readable (followed PEP8 style guide and GRASS Python<br>
> Scripting Library rules, converted python lists into dictionaries,<br>
> added comments, messages and warnings, etc.)<br>
> - Started writing the manual page<br>
> - Solved a problem with g.extension thanks to dev community<br>
> suggestions<br>
> - Discussed future improvements with the dev community<br>
> - Frequently added the new version of the code to my GitHub<br>
> repository [0]<br>
> - Shared progress with the community<br>
> <br>
> *2) What am I going to achieve for next week?*<br>
> <br>
> - Implement any change from discussions and feedback<br>
> - Test and fix bugs<br>
> - Finish writing the manual page<br>
> - Check the code with mentors<br>
> - Prepare deliverable for the evaluation<br>
> <br>
> *3) Is there any blocking issue?*<br>
> No at the moment.<br>
> <br>
> Here the links to my wiki page [1] and GitHub repository [0]<br>
> <br>
> Kind regards,<br>
> Roberta<br>
> <br>
> [0] <a href="https://github.com/RobiFag/GRASS_clouds_and_shadows" rel="noreferrer" target="_blank">https://github.com/RobiFag/GRA<wbr>SS_clouds_and_shadows</a><br>
> [1]<br>
> <a href="https://trac.osgeo.org/grass/wiki/GSoC/2018/CloudsAndShadowsDetection" rel="noreferrer" target="_blank">https://trac.osgeo.org/grass/w<wbr>iki/GSoC/2018/CloudsAndShadows<wbr>Detection</a><br>
<br>
</blockquote></div><br></div></div>