Opened 11 years ago

Closed 4 years ago

Last modified 4 years ago

#635 closed defect (fixed)

v.kernel and r.surf.fractal overwriting

Reported by: pcav Owned by: grass-dev@…
Priority: normal Milestone: 7.4.0
Component: Vector Version: svn-trunk
Keywords: v.kernel Cc:
CPU: All Platform: All

Description

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

Change History (12)

comment:1 in reply to:  description ; Changed 11 years ago by hamish

Component: defaultVector
CPU: UnspecifiedAll
Keywords: v.kernel added
Platform: UnspecifiedAll
Version: unspecifiedsvn-develbranch6

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

comment:2 in reply to:  1 ; Changed 8 years ago by 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

comment:3 in reply to:  2 Changed 8 years ago by 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.

comment:4 Changed 5 years ago by neteler

Milestone: 6.4.06.4.6

comment:5 Changed 4 years ago by annakrat

Milestone: 6.4.67.4.0
Version: svn-develbranch6svn-trunk

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?

comment:6 in reply to:  5 ; Changed 4 years ago by 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.

Last edited 4 years ago by martinl (previous) (diff)

comment:7 in reply to:  6 ; Changed 4 years ago by 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.

Last edited 4 years ago by martinl (previous) (diff)

comment:8 Changed 4 years ago by annakrat

In 70940:

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

comment:9 in reply to:  7 Changed 4 years ago by 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.

comment:10 Changed 4 years ago by annakrat

In 71088:

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

comment:11 Changed 4 years ago by annakrat

Resolution: fixed
Status: newclosed

Backported, so I am closing this.

comment:12 Changed 4 years ago by pcav

Thanks!

Note: See TracTickets for help on using tickets.