2018-06-11 10:29 GMT+02:00 Moritz Lennert <mlenn...@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 <robifagand...@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
>
>
_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev
