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

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.

[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

-----
best regards
Helmut
--
View this message in context: http://osgeo-org.1560.x6.nabble.com/Multi-resolution-Valley-Bottom-Flatness-MrVBF-index-r-valley-bottom-anyone-interested-in-code-review-tp5194799.html
Sent from the Grass - Users mailing list archive at Nabble.com.

* Helmut Kudrnovsky <hellik@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&gt;

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