Opened 8 years ago

Closed 3 years ago

#1444 closed defect (fixed)

g.parser does not filter for duplicate output map statements

Reported by: neteler Owned by: grass-dev@…
Priority: blocker Milestone: 6.4.6
Component: LibGIS Version: svn-releasebranch64
Keywords: parser Cc: lutra, pcav
CPU: All Platform: All

Description

A funny effect happens when the user states output map(s) several times due to copy-paste errors:

GRASS 6.4.2svn (nc_spm_08):~ > r.watershed elev_lid792_1m thresh=5000 drain=draindir_5K accum=accum_5K basin=basin_5K accum=accum_5K accum=accum_5K --o
SECTION 1a (of 5): Initiating Memory.
SECTION 1b (of 5): Determining Offmap Flow.
...
SECTION 5: Closing Maps.
 100%
Illegal filename. Character <,> not allowed.
WARNING: <accum_5K,accum_5K,accum_5K> is an illegal file name
WARNING: unable to open new accum map layer.
WARNING: category information for [accum_5K,accum_5K,accum_5K] in [neteler]
         missing or invalid
Illegal filename. Character <,> not allowed.
WARNING: can't write history information for [accum_5K,accum_5K,accum_5K]
GRASS 7.0.svn (nc_spm_08@grass70):~ > r.composite b=lsat7_2002_10 g=lsat7_2002_20 r=lsat7_2002_30 out=bla2 out=bla2
WARNING: Illegal filename <bla2,bla2>. Character <,> not allowed.
ERROR: <bla2,bla2> is an illegal file name

Change History (15)

comment:1 Changed 8 years ago by neteler

Summary: g.parser dpes not filter for duplicate output map statementsg.parser does not filter for duplicate output map statements

comment:2 Changed 8 years ago by hamish

Keywords: parser added

It would seem that by design (at least in 6.5) if you use a command line parameter more than once the parser concatenates them together with ',' as the f.sep.:

r.series in=roads in=fields out=both method=count
...
r.info -h both
...
   r.series input="roads,fields" output="both" method="count"

if the particular option does not allow multiple="YES", you get an error.

perhaps a way to handle this is for the parser to be more upfront about scanning for illegal map names &/or if multiple=NO?

Hamish

comment:3 in reply to:  2 Changed 8 years ago by glynn

Replying to hamish:

It would seem that by design (at least in 6.5) if you use a command line parameter more than once the parser concatenates them together with ',' as the f.sep.:

Correct; this feature has existed since forever, but isn't widely known. It can be useful in conjunction with e.g. xargs, as generating multiple options is easier than trying to glue the values together into a single option.

if the particular option does not allow multiple="YES", you get an error.

perhaps a way to handle this is for the parser to be more upfront about scanning for illegal map names &/or if multiple=NO?

Try r48205 (7.0).

comment:4 Changed 8 years ago by neteler

Resolution: fixed
Status: newclosed

Very nice (GRASS 7):

r.composite b=lsat7_2002_10 g=lsat7_2002_20 r=lsat7_2002_30 out=bla2 out=bla2
...
ERROR: Option <output> does not accept multiple answers

I quickly tested r.series, too, seems to behave as it should. Closing & thanks.

comment:5 in reply to:  4 ; Changed 8 years ago by martinl

Resolution: fixed
Status: closedreopened

Replying to neteler:

I quickly tested r.series, too, seems to behave as it should. Closing & thanks.

Milestone: 6.4.2. I would expect backport to devbr6 and relbr64, or not?

comment:6 in reply to:  5 Changed 8 years ago by glynn

Replying to martinl:

Milestone: 6.4.2. I would expect backport to devbr6 and relbr64, or not?

Probably. Aside from the "enhancement", r48205 also contains a bug fix; appending answers to the default argument would discard any values specified without the option=. IOW:

r.series map1,map2 input=map3,map4

was treated as:

r.series input=map3,map4

rather than:

r.series input=map1,map2,map3,map4

comment:7 Changed 8 years ago by neteler

Resolution: fixed
Status: reopenedclosed

Backported to 6.4.svn and 6.5.svn in r48233 and r48234.

comment:8 Changed 7 years ago by neteler

Resolution: fixed
Status: closedreopened

This fix seems to introduce a new problem:

GRASS 6.4.2svn (nc_spm_08):~ > v.what.vect myhospitals qvect=urbanarea
column=urb_name qcolumn=NAME =silly
 100%
...

GRASS 6.4.2svn (nc_spm_08):~ > v.what.vect myhospitals qvect=urbanarea
column=urb_name qcolumn=NAME =silly ==d
 100%
 ...

GRASS 6.4.2svn (nc_spm_08):~ > v.what.vect myhospitals qvect=urbanarea
column=urb_name qcolumn=NAME =silly ==d layer=1
 100%

According to #1477 this was not the case in 6.4.1.

comment:9 Changed 7 years ago by lutra

GRASS 6.4.2RC249259 (installed with osgeo4w) seems to still accept such wrong parameters. Example:

C:\>g.version
GRASS 6.4.2RC249259 (2011)
C:\>v.what.vect vector=pontos@teste2 =point layer=1 
column=ETICHETTA qvector=prov@teste2 =area qlayer=1 qcolumn=PROVINCIA
 100%
 100%
 100%
100 categories read from the map
100 categories exist in the table
100 categories read from the map exist in the table
100 records updated
v.distance complete.

comment:10 in reply to:  8 Changed 7 years ago by glynn

Replying to neteler:

This fix seems to introduce a new problem:

This should be fixed in r49481

comment:11 Changed 7 years ago by neteler

Cc: lutra pcav added

I have locally backported it and it helps:

GRASS 6.4.2svn (nc_spm_08):~ > v.what.vect myhospitals qvect=urbanarea column=urb_name qcolumn=NAME =silly ==d layer=1
Sorry <=silly> is not a valid option
Sorry <==d> is not a valid option

any risk to submit this backport to SVN?

comment:12 Changed 7 years ago by neteler

Backported to 6.5 in r49509. I suggest to also backport to 6.4 (for 6.4.2) if no risk is involved.

comment:13 Changed 7 years ago by neteler

Milestone: 6.4.26.4.3

Is r49481 save to be backported to 6.4?

comment:14 in reply to:  13 Changed 3 years ago by neteler

Milestone: 6.4.36.4.6
Priority: minorblocker
Version: 6.4.1svn-releasebranch64

Replying to neteler:

Is r49481 save to be backported to 6.4?

Marked as blocker for G6

comment:15 Changed 3 years ago by mlennert

Resolution: fixed
Status: reopenedclosed

In 69209:

parser: fix #1444 (backport from devel6)

Note: See TracTickets for help on using tickets.