Opened 15 years ago

Closed 15 years ago

#739 closed defect (wontfix)

v.mkgrid parameter needs two values but is multiple=NO

Reported by: rsbivand Owned by: grass-dev@…
Priority: normal Milestone: 6.4.0
Component: Vector Version: 6.4.0 RCs
Keywords: v.mkgrid Cc:
CPU: Unspecified Platform: Unspecified

Description

When run from the command line, v.mkgrid map=xxx grid=100,50 works (main.c, line 74), but in wxpython and execGRASS() in spgrass6 in R, the --interface-description of grid->multiple = NO is respected, and only one value can be passed. This then fails (in wxpython here) with:

v.mkgrid --overwrite map=grd grid=100                                           
ERROR: option <grid> must be provided in multiples of 2
       You provided 1 items:
       100

In R (using --interface-description):

> execGRASS("v.mkgrid", flags="overwrite", parameters=list(map="grd", grid=as.integer(100)))

ERROR: option <grid> must be provided in multiples of 2
       You provided 1 items:
       100

or:

> execGRASS("v.mkgrid", flags="overwrite", parameters=list(map="grd", grid=rep(as.integer(100), 2)))
Error in doGRASS(cmd, flags = flags, parameters = parameters) : 
  Parameter <grid> has multiple values

This conflicts with the single comma in grid->key_desc, which implies two values, but grid->multiple = NO. The same seems to apply to box=. It looks as though there is no conflict on the command line when multiple = NO provided that the number of values agrees with # commas in key_desc plus one, but that this does not work when --interface description is used.

Change History (5)

comment:1 by rsbivand, 15 years ago

Component: defaultVector
Version: unspecified6.4.0 RCs

in reply to:  description comment:2 by neteler, 15 years ago

Replying to rsbivand:

When run from the command line, v.mkgrid map=xxx grid=100,50 works (main.c, line 74), but in wxpython and execGRASS() in spgrass6 in R, the --interface-description of grid->multiple = NO is respected, and only one value can be passed. This then fails (in wxpython here) with:

v.mkgrid --overwrite map=grd grid=100                                           
ERROR: option <grid> must be provided in multiples of 2
       You provided 1 items:
       100

Yes, it must be 100,100 (the user has to specify two values).

In R (using --interface-description):

> execGRASS("v.mkgrid", flags="overwrite", parameters=list(map="grd", grid=as.integer(100)))

ERROR: option <grid> must be provided in multiples of 2
       You provided 1 items:
       100

here you need to use

... grid=paste(as.integer(100), as.integer(100), sep=",")

or:

> execGRASS("v.mkgrid", flags="overwrite", parameters=list(map="grd", grid=rep(as.integer(100), 2)))
Error in doGRASS(cmd, flags = flags, parameters = parameters) : 
  Parameter <grid> has multiple values

Yes, because

> rep(as.integer(100), 2)
[1] 100 100

which is space but not comma separated. Indeed you need to use:

> paste(as.integer(100), as.integer(100), sep=",")
[1] "100,100"

This conflicts with the single comma in grid->key_desc, which implies two values, but grid->multiple = NO. The same seems to apply to box=. It looks as though there is no conflict on the command line when multiple = NO provided that the number of values agrees with # commas in key_desc plus one, but that this does not work when --interface description is used.

I don't think that this is a conflict: two comma separated values are taken as one "string". Instead, e.g. in r.series, multiple input maps are possible which are multiple = YES, hence, they are treated as separate tokens.

comment:3 by hamish, 15 years ago

Keywords: v.mkgrid added

Roger wrote: (please reply in-ticket)

The problem is that the GRASS parser adopts an illogical
view of multiple=NO, and overrides it when there is a comma
in keydesc. This means that I am obliged to make parseGRASS()
even more byzantine than before, to admit multiple integers
and floats where multiple=NO and the length of the R object
of the correct storage mode is > 1 (strings can be obfuscated
in the way you suggest). I'll try to mimic the IMHO very bad
behaviour of the parser.

Couldn't multiple take values YES, NO, or the integer count
required if the count is known, as here? multiple=2 would be
easy to parse.

Repeating, the 6.4 wxpython menu entry Vector -> Generate
grid is broken for me 6.4.0svn, but not current, because the
widget for entering grid only accepts a single integer (as per
--interface-description).

In current 6.4.0svn the widget has been changed to a text
widget, so that's why it works, but does not check for integer:

v.mkgrid --overwrite map=grd@rsb grid=99.3de,101.5abc

also runs, probably by rounding to integer.

I'll try to work around the parsing muddle, but will check
that the values given are both integer.

Roger

It's not illogical; it's just unintuitive in the wxGUI as some critical information is not being displayed.

It should take it's cue from both opt->key_desc and opt->multiple.

e.g. r.what with multiple=YES has east_north=x1,y1[,x2,y2[...]]

if you give 3 values it complains that it wants a multiple of two.

or d.legend opt7->key_desc = "bottom,top,left,right"; but there multiple sets of four = NO;

this works quite well in the C parser.

you are right, in the wxGUI for v.mkgrid it gives you no indication that you should do "rows,columns" for the required grid option. It just says (grid, integer)

In the tcltk GUI you get a bit more info:

(grid, rows,columns, required)

coor= is perhaps a more obvious example.

(coor, x,y, optional)

the help page, --help USAGE is the best:

grid=rows,columns [coor=x,y]

grid=rows,columns

Number of rows and columns in grid

that the parser doesn't check by type as well as it could is another issue for another ticket.

Hamish

comment:4 by hamish, 15 years ago

in latest 6.4svn I can successfully run v.mkgrid from the wxGUI with:

v.mkgrid map=testgrid grid=5,5

... as long as I know to put 5,5 in the "grid" entry.

Hamish

comment:5 by rsbivand, 15 years ago

Resolution: wontfix
Status: newclosed

OK, I'll back off, too much legacy stuff depends on multiple keydescs being coded with multiple as NO. I've added keydesc detection and parsing to parseGRASS() in spgrass6, since the alternative would break things in GRASS itself.

Note: See TracTickets for help on using tickets.