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