Michael Barton wrote:
Testing our r.prominence, I found that it will not locate any raster
files for input in the GUI. Looking at the code, it calls fcell.
According to element_list, this is a valid element, but it is not
clear how it is called, since it is called a "support_element". The
program requires a floating point raster, so specifying it in the GUI
is a good idea.
cell:rast:raster:raster files
cellhd:header
cats:category
colr:color
hist:history
cell_misc:misc
fcell:fcell
g3dcell:g3dcell
So, does the GUI parser recognize support_element items? If so, how
are they to be called in the the section that specifies the GUI for a
module?
Checking for the existence of a raster is done by looking for the
.../cell/<mapname> file. Even FP maps have this file, although it has
zero length.
r.prominence appears to have been written without any reference to the
documentation or existing code. Two points stand out:
1.
parm.input->gisprompt = "old,fcell";
parm.output->gisprompt = "fcell";
These should be "old,cell,raster" and "new,cell,raster" respectively.
There is no way to instruct the parser to check for an FP map; you
have to do this yourself.
2.
if ((flag.localnorm->answer) || (flag.globalnorm->answer)) {
G_remove("cats", parm.output->answer);
G_remove("cell", parm.output->answer);
G_remove("cell_hd", parm.output->answer);
G_remove("cell_misc", parm.output->answer);
G_remove("colr", parm.output->answer);
G_remove("colr2", parm.output->answer);
G_remove("fcell", parm.output->answer);
G_remove("hist", parm.output->answer);
G_rename("cats", sysstr, parm.output->answer);
G_rename("cell", sysstr, parm.output->answer);
G_rename("cell_hd", sysstr, parm.output->answer);
G_rename("cell_misc", sysstr, parm.output->answer);
G_rename("colr", sysstr, parm.output->answer);
G_rename("colr2", sysstr, parm.output->answer);
G_rename("fcell", sysstr, parm.output->answer);
G_rename("hist", sysstr, parm.output->answer);
This is bogus. If there is a valid reason to remove/rename maps, it
should spawn g.remove/g.rename.
Some other issues:
mapset = G_calloc(512, sizeof(char));
mapset = G_find_cell(parm.input->answer, "");
The first line is pointless, and just wastes memory. The second line
should probably be G_find_cell2().
G_fatal_error
("Input map is not a floating point map.\nOnly integer maps are allowed as input maps.");
Not localised, too verbose, and self-contraditory.
if (!G_legal_filename(parm.output->answer)) {
G_fatal_error("%s is not a legal filename for an output map.\n",
parm.output->answer);
Invalid use of G_legal_filename(), which is supposed to operate upon
individual components, not complete map names (map@mapset is not a
legal filename but is valid here, so long as the mapset is the current
mapset).
if (!flag.quiet->answer) {
G_percent(y, nrows - 1, 1);
fflush(stdout);
}
The fflush(stdout) is a bit pointless, given that G_percent() writes
to stderr (which is unbuffered). If a fflush() was needed, it would be
done by G_percent().
free(mapset);
The return value from G_find_* may be shared and shouldn't be freed.
--
Glynn Clements <glynn@gclements.plus.com>