v.kernel and r.surf.fractal overwriting

[pcav]

Several GRASS modules in QGIS (I found v.kernel and r.surf.fractal) don't check if a layer exist before overwriting it. It appears a GRASS bug, not a QGIS one. See also ​http://trac.osgeo.org/qgis/ticket/1616

[hamish]

Replying to pcav:

Several GRASS modules in QGIS (I found v.kernel and r.surf.fractal) don't check if a layer exist before overwriting it.

the problem is that their option description miss the gisprompt field which says if it will be and old or new map. For new maps the parser does the overwrite check.

AFAICT Martin fixed this for r.surf.fractal in December by changing the custom option definition to using a standard defined output one, but for v.kernel it is still missing.

the problem with v.kernel is that the output option is used somehow for both raster and vector maps. And since the gisprompt definition can only be set to one or the other it is not set at all. In this case the module should include its own 'already exists' check and check for --overwrite to decide if that will be a fatal error or just a warning.

Hamish

[mlennert]

Replying to hamish:

the problem with v.kernel is that the output option is used somehow for both raster and vector maps. And since the gisprompt definition can only be set to one or the other it is not set at all. In this case the module should include its own 'already exists' check and check for --overwrite to decide if that will be a fatal error or just a warning.

Just was confronted to this problem (no check on overwrite) in v.kernel (all versions) and found this very old bug report. I think the solution is rather that output raster or vector maps should become optional with a check that at least one of them is given. Then you can use the gisprompt for each type and thus trigger the check.

Or is there something that counter-indicates such a solution ?

Moritz

[glynn]

Replying to mlennert:

I think the solution is rather that output raster or vector maps should become optional with a check that at least one of them is given. Then you can use the gisprompt for each type and thus trigger the check.

Indeed. A single option shouldn't be used for different purposes, which includes different output formats.

[annakrat]

This bug is still present. The solution is to split the options. Ideally, we keep 'output' for raster output and the more specialized vector output could be something like 'net_output'. I don't think we can do it now this way for backwards compatibility reasons. Or we can say that since most users are interested in the raster output anyway, we do the change for 7.4. Any opinion?

[martinl]

Replying to annakrat:

This bug is still present. The solution is to split the options. Ideally, we keep 'output' for raster output and the more specialized vector output could be something like 'net_output'. I don't think we can do it now this way for backwards compatibility reasons. Or we can say that since most users are interested in the raster output anyway, we do the change for 7.4. Any opinion?

+1 for introducing new option net_output.

[martinl]

Replying to martinl:

+1 for introducing new option net_output.

The module could set net_output based on output if net is given and print warning. This could avoid breaking backward compatibility.

[annakrat]

In 70940:

v.kernel: add separate option for network density map, see #635

[annakrat]

Replying to martinl:

Replying to martinl:

+1 for introducing new option net_output.

The module could set net_output based on output if net is given and print warning. This could avoid breaking backward compatibility.

Right, done in r70940.

[annakrat]

In 71088:

v.kernel: add separate option for network density map, see #635 (merge from trunk, r70940)

[annakrat]

Backported, so I am closing this.

[pcav]

Thanks!