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

[Moritz Lennert]

Hi Roberta,

Thanks for your report and your great work !

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 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)

	- 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]

- 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.


- Some remarks on your handling of temporary maps:

	- You use

		processid = "{:.2f}".format(time.time())
		processid = processid.replace(".", "_")

	why not simply:

		processid = os.getpid() ?

	- 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) 

- 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.


Moritz

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