[GRASS-user] Multi-resolution Valley Bottom Flatness (MrVBF) index (r.valley.bottom): anyone interested in code review?

[Nikos Alexandris]

* Helmut Kudrnovsky <hellik at web.de> [2015-03-21 16:10:49 -0700]:

> hi,
> 
> the calculation of the Multi-resolution Valley Bottom Flatness (MrVBF) index
> (John C. Gallant and Trevor I. Dowling 2003. [1]) in GRASS GIS [2] is a work
> in progress ...
> 
> anyone interested in a code review of r.valley.bottom [2]? the original
> paper can be shared if wanted.

Hallo Helmut!

I didn't read all of it. One thing, however, that can be changed is to
use temporary map names with some specified prefix. In the end, these
can be removed with one command only.

This will make the script easier to read and handle in general.

Specifically, lines 653 [0] to 671, can be replaced with one command. If
you should (?) keep using the map names as they are now, you could create 
a list for them and just feed the list to g.remove directly.


[0] <http://trac.osgeo.org/grass/browser/grass-addons/grass7/raster/r.valley.bottom/r.valley.bottom.py#L653>


I am not sure whether my practice is a good one, but that's what I try
to follow so far:



In the beginning:

--%<---
import atexit
--->%--



Somewhere after and before main:

--%<---
# helper function
def cleanup():
    """
    Clean up temporary maps
    """
    grass.run_command('g.remove', flags='f', type="rast",
                      pattern='tmp.{pid}*'.format(pid=os.getpid()), quiet=True)
--->%--- 



Then, in main:

--%<---
def main():
    ...
    # for Temporary files
    global tmp  # useful in case of defining temporary map names inside helper functions
    tmpfile = grass.tempfile()  # replace with os.getpid?
    tmp = "tmp." + grass.basename(tmpfile)  # use its basename
    ...
--->%--

You can now create temporary file names, inside the main() function or else, for example like:

---%<--
tmp_F1 = tmp + '.F1'  # some comment
tmp_F2 = tmp + '.F2'  # this is for map F2
tmp_MRVBF4 = tmp + '.MRVBF4' # this is the output
--->%--

and so on.



Just before the end of the script, you can g.rename the temporary maps you want to keep (meaning the maps which are supposed to be a valid output of the script).
Below, "output" can be, of course, a user-defined output map name

--%<---
# (re)name end product
run("g.rename", rast=(tmp_MRVBF4, output))  
--->%--



And, finally, in the end:

--%<---
if __name__ == "__main__":
    options, flags = grass.parser()
    atexit.register(cleanup)
    sys.exit(main())
--->%--


Nikos



> [1] John C. Gallant and Trevor I. Dowling 2003.
>            A multiresolution index of valley bottom flatness for mapping
> depositional areas.
>            WATER RESOURCES RESEARCH, VOL. 39, NO. 12, 1347,
> doi:10.1029/2002WR001426, 2003
> 
> [2] r.valley.bottom:
> http://trac.osgeo.org/grass/browser/grass-addons/grass7/raster/r.valley.bottom