v.vol.rst WARNING in GRASS 7 still says cellout and cellinp rasters

[rorschach]

A very minor issue. GRASS 7 changed the names of cellinp and cellout parameters of the v.vol.rst module to cross_input and cross_output respectively but when the module issues a warning it still says:

WARNING: Unable to create cellout raster map without cellinp (emphasis supplied)

This may be minor but I think it throws off a user because there is no cellout or cellinp option anymore in GRASS 7.

I haven't checked if the same goes for other WARNING/ERROR messages for other modules. The main source code probably needs a little proofreading to avoid simple defects like this. I would volunteer but I don't have access to the main svn (yet).

Thank you.

[hellik]

Replying to rorschach:

I haven't checked if the same goes for other WARNING/ERROR messages for other modules. The main source code probably needs a little proofreading to avoid simple defects like this. I would volunteer but I don't have access to the main svn (yet).

you can have a look online at the code of trunk and all the branches here:

​https://trac.osgeo.org/grass/browser/grass

or do a svn checkout, see ​svn checkout

[rorschach]

Checked the source, found the problem on line 640 of the module's main.c file which reads:

G_warning(_("Unable to create cellout raster map without cellinp"));

Can someone change this to:

G_warning(_("Unable to create cross_output raster map without cross_input"));

for consistency?

Thanks.

[wenzeslaus]

Thanks. A better fix is using the names as defined in the Option structure by attribute key (example from r.slope.aspect​:

G_fatal_error(_("%s=%s - must be a positive number"), parm.zfactor->key, parm.zfactor->answer);

Can you please also check if there are other places like this, e.g. using grep cell and create a patch using svn diff?

[neteler]

Attempt done in r69439: if ok to be backported

[mlennert]

Replying to neteler:

Attempt done in r69439: if ok to be backported

Actually, the test tests for the existance of both maps:

if ((cellinp != NULL) && (cellout != NULL)) {

so the message will pop up if either of them is missing or if both are missing, so this is not very helpful. Especially when you don't give either of them, you get:

ATTENTION: Unable to create <(null)> raster map without cross_input raster
           map being specified

I think this should be handled by parameter dependency rules for the parser, and not by the code. Something like:

G_option_collective(parm.cellinp, parm.cellout)

[mlennert]

diff to implement options dependency checks through parser

[mlennert]

Replying to mlennert:

Replying to neteler:

Attempt done in r69439: if ok to be backported

Actually, the test tests for the existance of both maps:

if ((cellinp != NULL) && (cellout != NULL)) {

so the message will pop up if either of them is missing or if both are missing, so this is not very helpful. Especially when you don't give either of them, you get:

ATTENTION: Unable to create <(null)> raster map without cross_input raster
           map being specified

I think this should be handled by parameter dependency rules for the parser, and not by the code. Something like:

G_option_collective(parm.cellinp, parm.cellout)

Try the attached patch which implements the above and other checks that were done in the code.

[martinl]

Still relevant?