[GRASS-dev] significant change in g.parser behavior for GRASS 7

[Vaclav Petras]

On Thu, Sep 25, 2014 at 11:56 PM, Michael Barton <Michael.Barton at asu.edu>
wrote:

> Thanks for the explanation Glynn. So this is a byproduct of changes for
> other goals. It is good to learn that a work around can be simply
> re-odering the options.


I would recommend not to work around thing like this. The behavior might be
changed in the future and you will be in the same or worse situation then
now.

More importantly, according to my understanding, reordering will not help
you.

I created four scripts. Each has two parameters/options "elevation" and
"slope". Script test_lower_lower has both options lowercase,
test_upper_upper has both options with uppercase letter etc. The output of
the script are the options (as Python dictionary).

Standard case:

./test_lower_lower.py elevation=abc slope=cde
{'slope': 'cde', 'elevation': 'abc'}

Both upper case, error reported for the second option. Error is not really
informative but it fails.

./test_upper_upper.py Elevation=abc Slope=cde
ERROR: Sorry <Slope=cde> is not a valid option
ERROR: Required parameter <Slope> not set:
    (Name for output slope raster map)

Same case as above, second option is not accepted (in this case the message
at least applies to the first wrong option):

./test_lower_upper.py elevation=abc Slope=cde
ERROR: Sorry <Slope=cde> is not a valid option
ERROR: Required parameter <Slope> not set:
    (Name for output slope raster map)

Only the first option is upper case and is accepted but the result is not
what you would like to get:

./test_upper_lower.py Elevation=abc slope=cde
{'slope': 'cde', 'Elevation': 'Elevation=abc'}

However, this is what r.mapcalc benefits from, for maximum compatibility in
the way the example above is showing and in standard cases in the way below:

./test_upper_lower.py abc slope=cde
{'slope': 'cde', 'Elevation': 'abc'}


> This makes it easier to maintain chaining between scripts or calling
> modules from other programs.


The changes are not more serious then changes in module parameters/options
and flags or module names and there is a lot of these changes between 6 and
7. Sorry, I really don't see much difference between other API changes and
this one.


> This behavior is still a potential gotcha for anyone writing scripts,
> however. Sometimes it will give an ambiguous error and a similar script
> with a different order of options will not.
>
> And this is exactly the reason why the lowercase should be enforced as
soon as possible in the parsing process or at least a better error message
should be provided. It would be good to avoid tests which would slow down
the correct scripts but I don't know if it would actually make any
difference.

Michael
> ____________________
> C. Michael Barton
> Director, Center for Social Dynamics & Complexity
> Professor of Anthropology, School of Human Evolution & Social Change
> Head, Graduate Faculty in Complex Adaptive Systems Science
> Arizona State University
>
> voice:  480-965-6262 (SHESC), 480-965-8130/727-9746 (CSDC)
> fax: 480-965-7671 (SHESC),  480-727-0709 (CSDC)
> www: http://www.public.asu.edu/~cmbarton, http://csdc.asu.edu
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> On Sep 25, 2014, at 4:40 PM, Glynn Clements <glynn at gclements.plus.com>
> wrote:
>
> >
> > Michael Barton wrote:
> >
> >> I just learned that using upper case characters for a GRASS 7 module
> >> option means that g.parser will not recognize it.
> >
> > Technically, the change is to G_parser(); g.parser simply inherits the
> > behaviour.
> >
> >> That is for module. �r.foo" with options �Abc� and �def�, the
> >> command
> >>
> >> r.foo Abc=1 def=2
> >>
> >> will return an error <Abd=1> is not a valid option.
> >
> > No it won't.
> >
> > However, this will produce that error:
> >
> >       r.foo def=2 Abc=1
> >
> > If the first argument doesn't match the required syntax (e.g. because
> > of an upper-case character in the option name), it will be treated as
> > the value to the default (first) option. IOW, assuming that "Abc" is
> > the first option, your example:
> >
> >       r.foo Abc=1 def=2
> >
> > will be treated as:
> >
> >       r.foo Abc=Abc=1 def=2
> >
> >> GRASS 6 does not produce an error in this case. The module will run
> fine.
> >>
> >> This breaks all kinds of existing scripts from GRASS6, as well as
> >> scripts that are designed to be chained together. I�ve never seen
> >> any discussion of this. Perhaps I missed it because I was in the
> >> field or something. Is there a reason for this significant change of
> >> g.parser behavior?
> >
> > The changes in question are r32259 and r32261, from July 2008, and
> > r33576 from September 2008.
> >
> > The rationale was related to changing r.mapcalc to use G_parser().
> >
> > Following that change, if r.mapcalc is run as e.g.
> >
> >       r.mapcalc "map=expression"
> >
> > G_parser() will complain that there is no option named "map".
> >
> > The preferred way of avoiding this issue is to place spaces around the
> > "=" sign; a space before the "=" guarantees that the argument will not
> > be treated as having the option=value form, and will instead be
> > treated as the value to the default option (expression=).
> >
> > In order to maximise compatibility with the previous version, the test
> > for whether an argument has the option=value form was restricted. Any
> > argument not conforming to the (newly-restricted) syntax will be
> > treated as the value of the expression= option. The more restrictive
> > the syntax, the fewer expressions will be mistaken for option=value.
> >
> > Originally, any argument containing an "=" was considered as having
> > the option=value form.
> >
> > r32259 changed this to require that the character immediately before
> > the first "=" was not a space or tab. r32261 refined it to require the
> > character to be a lower-case letter or a digit. r33576 further refined
> > this so that the entire portion before the first "=" must consist
> > solely of lower-case letters, digits and underscores, and the
> > character immediately before the "=" must not be an underscore.
> >
> > With one exception; all option names used in GRASS' own modules and
> > scripts already satisfied the new restrictions. The sole exception was
> > r.terraflow, which had the STREAM_DIR= option renamed to stream_dir=
> > in r32260.
> >
> > --
> > Glynn Clements <glynn at gclements.plus.com>
>
> _______________________________________________
> grass-dev mailing list
> grass-dev at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/grass-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/grass-dev/attachments/20140926/0ab6c380/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test_upper_lower.py
Type: text/x-python
Size: 514 bytes
Desc: not available
URL: <http://lists.osgeo.org/pipermail/grass-dev/attachments/20140926/0ab6c380/attachment-0001.py>