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

[Roberta Fagandini]

2018-06-11 10:29 GMT+02:00 Moritz Lennert <mlennert at club.worldonline.be>:

> Hi Roberta,
>
> Thanks for your report and your great work !
>

Thank to you for your support!


>
> Here are a few remarks:
>
> - You have a -t flag with description "Do not delete temporary files".
>   Is this only for debugging purposes (and which temporary files are
>   created), or does this have any real use for the user ? If the
>   former, this should probably not be part of the module UI, if the
>   latter, it should be explained in the man page.
>

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.
What do you think? Could it be useful?


>
> - I have several comments concerning your input_file parameter
>
>
>         - The syntax of this file has to be explained in the man page,
>           including the precise variable names (blue, not Blue, not
>           BLUE, etc)
>

Of course! I have already added the explanation this morning.


>
>         - The code for reading this file can probably be simplified
>           quite easily by using something like this:
>
>         for line in file(input_file):
>                 a = line.split('=')
>                 if len(a) <> 2 or a[0] not in ['blue', 'red', etc ]:
>                         gscript.fatal("Syntax error in the txt file.")
>                 a[1] = a[1].strip()
>                 bands[a[0]] = a[1]
>

Thank you! this is a better solution than mine. It was a first experiment
but I have already implemented your suggestion.


>
> - Again a (minor) remark on readability of the code. When you use named
>   variables in format(), I would suggest that you use the actual names.
>   i.e. instead of
>
>         gscript.mapcalc('{r} = 1.0 * ({a})/{c}'.format(
>                                 r=("{}_{}".format(b, d)),
>                                 a=b, c=scale_fac),
>                                 overwrite=True)
>
> why not use this:
>
>         gscript.mapcalc('{r} = 1.0 * ({b})/{scale_fac}'.format(
>                                 r=("{}_{}".format(b, d)),
>                                 b=b, scale_fac=scale_fac),
>                                 overwrite=True)
>
> ?
>
> I think it makes the formula more easily readable.
>

Yes, you are right! Done!


>
>
> - Some remarks on your handling of temporary maps:
>
>         - You use
>
>                 processid = "{:.2f}".format(time.time())
>                 processid = processid.replace(".", "_")
>
>         why not simply:
>
>                 processid = os.getpid() ?
>

I didn't know this function, it seems to be cleaner than mine..thank you,
added!


>
>         - Instead of having to check for the type of every single map
>           in the cleanup function, you could include the type in the
>           info, i.e. something like this:
>
>                 r = 'raster'
>                 v = 'vector'
>
>                 tmp["cloud_v"] = ["cloud_v_" + processid, v]
>                 (I personally use "cloud_v_%i" % processid; not sure
>                 what is preferable)
>
>         which then allows you to do something like this in cleanup():
>
>         for temp_map, maptype in tmp:
>                 if gscript.find_file(temp_map, element=maptype)['name']:
>                     gscript.run_command('g.remove',
>                                         flags='f',
>                                         type=maptype,
>                                         name=temp_map,
>                                         quiet=True)
>

I tried to implement this solution but in this way, I get a warning message:

 WARNING: Illegal filename <cloud_v_8048,vector> etc.

I tried different ways and I looked for something similar but I can not
find how to add the map type in the info.


>
> - In its final version your man page should contain an example,
>   ideally a complete example starting with i.sentinel.download and
>   going all the way to i.sentinal.mask in order to show the use of the
>   module within a reproducible workflow, best using a North Carolina
>   example to fit with the demo dataset.
>

I had already thought about making a complete example. I hope to prepare
and add this first part by the end of this week.


>
>
> Moritz
>

Roberta


>
> Le Sun, 10 Jun 2018 10:58:37 +0200,
> Roberta Fagandini <robifagandini at gmail.com> a écrit :
>
> > 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 fourth week of coding.
> >
> > *1) What did I complete this week?*
> >
> >    - Implemented some changes from dev feedback (e.g. added a new
> > flag to manage the two procedure separately and added the text file
> > option to specify input bands)
> >    - Tested the modified python script and fixed bugs (e.g. solved a
> > bug for -s flag)
> >    - Created a real complete GRASS GIS module that can be installed
> > with g.extension
> >    - Finished implementing the GUI
> >    - Tested the GUI and fixed bugs
> >    - Cleaned up the code from the style point of view in order to
> > make it more readable (followed PEP8 style guide and GRASS Python
> > Scripting Library rules, converted python lists into dictionaries,
> > added comments, messages and warnings, etc.)
> >    - Started writing the manual page
> >    - Solved a problem with g.extension thanks to dev community
> > suggestions
> >    - Discussed future improvements with the dev community
> >    - Frequently added the new version of the code to my GitHub
> > repository [0]
> >    - Shared progress with the community
> >
> > *2) What am I going to achieve for next week?*
> >
> >    - Implement any change from discussions and feedback
> >    - Test and fix bugs
> >    - Finish writing the manual page
> >    - Check the code with mentors
> >    - Prepare deliverable for the evaluation
> >
> > *3) Is there any blocking issue?*
> > No at the moment.
> >
> > Here the links to my wiki page [1] and GitHub repository [0]
> >
> > Kind regards,
> > Roberta
> >
> > [0] https://github.com/RobiFag/GRASS_clouds_and_shadows
> > [1]
> > https://trac.osgeo.org/grass/wiki/GSoC/2018/CloudsAndShadowsDetection
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/grass-dev/attachments/20180611/7da7c90b/attachment-0001.html>