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

Fri Sep 26 10:10:04 PDT 2014

It is good to see these tests. So order does not fix it like Glynn implied. It also gives different results depending on order and capitalization.

IMHO, this is a fairly serious problem. I’m OK with lower case being enforced but the way to do this that causes the least damage for porting scripts from GRASS 6 to GRASS 7 is for GRASS 7 to simply recognize all option names as lower case. That is option = lower(option).

The reason we found this is because we are porting to GRASS 7 a script that has worked in GRASS 6. We started getting the misleading error messages that you documented below.

At the very least, the error generated needs to be something along the lines of  “upper case characters are not allowed for option names in GRASS modules”

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
Tempe, AZ  85287-2402
USA

voice:  480-965-6262 (SHESC), 480-965-8130/727-9746 (CSDC)
fax:          480-965-7671(SHESC), 480-727-0709 (CSDC)
www:  http://csdc.asu.edu, http://shesc.asu.edu
http://www.public.asu.edu/~cmbarton

On Sep 26, 2014, at 9:59 AM, Vaclav Petras <wenzeslaus at gmail.com<mailto:wenzeslaus at gmail.com>> wrote:

On Thu, Sep 25, 2014 at 11:56 PM, Michael Barton <Michael.Barton at asu.edu<mailto: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<tel:480-965-6262> (SHESC), 480-965-8130<tel:480-965-8130>/727-9746 (CSDC)
fax: 480-965-7671<tel:480-965-7671> (SHESC),  480-727-0709<tel:480-727-0709> (CSDC)
www: http://www.public.asu.edu/~cmbarton, http://csdc.asu.edu<http://csdc.asu.edu/>

On Sep 25, 2014, at 4:40 PM, Glynn Clements <glynn at gclements.plus.com<mailto: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<mailto:glynn at gclements.plus.com>>

_______________________________________________
grass-dev mailing list
grass-dev at lists.osgeo.org<mailto:grass-dev at lists.osgeo.org>
http://lists.osgeo.org/mailman/listinfo/grass-dev

<test_upper_lower.py>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/grass-dev/attachments/20140926/986263c5/attachment-0001.html>