[GRASS-dev] [GRASS GIS] #2409: last call for options keys consolidation

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------
Before releasing G7 we should check all the modules and use the
standardized options as much as possible. Changing key of an option or a
flag will be not possible after releasing GRASS 7.0.0. For that reason I
prepared simple Python script which prints for each GRASS module the list
of options keys and descriptions
source:sandbox/martinl/print_module_parameters.py. A generated list for
GRASS 7.0.0 attached: attachment:module_params_overview.txt.

Please feel free to go through the list and comment her all remaining
issues where a parameter or a flag key should be changed.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by cmbarton):

This is very helpful Martin. Thanks much. It will take a lot of time to go
through. A quick glance indicates a lot of inconsistency in flags and even
arguments. This is not surprising given the long evolution of GRASS.

So a question to all is how much do we want to standardize here. As Martin
says, once GRASS 7.0.0 is released we will not be able to change anything
until GRASS 8--a very long time down the road. But changes we do now can
break any scripts written for GRASS 6 of course.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by neteler):

Replying to [ticket:2409 martinl]:
> I prepared simple Python script which prints for each GRASS module the
list of options keys and descriptions
source:sandbox/martinl/print_module_parameters.py. A generated list for
GRASS 7.0.0 attached: attachment:module_params_overview.txt.

Very good. I have added an extra script in your sandbox
("print_module_parameters_csv.py") which still stops looping at "g.parser"
which does not have flags/params.
No idea how to solve that, it will be a simple Python statement.

Idea: Table export allows to load it into a spreadsheet and sort by
description
to catch the too similar ones and streamline them.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by martinl):

Replying to [comment:2 neteler]:
> Very good. I have added an extra script in your sandbox
("print_module_parameters_csv.py")

good idea, probably we could move scripts to `addons/tools`?

> which still stops looping at "g.parser" which does not have
flags/params.
> No idea how to solve that, it will be a simple Python statement.

really? `g.parsers` seems to be excluded...

{{{
g.mremove€exclude€Map name exclusion pattern (default: none)
g.pnmcomp€input€Name of input file(s)
}}}

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by neteler):

Excluded yes, but then it breaks:

(note: I use the Euro sign as field separator for the CSV file since most
of the chars are taken)

{{{
GRASS 7.1.svn (nc_spm_08_grass7):~/software/grass-sandbox/martinl > python
print_module_parameters_csv.py
...
g.mremove€f€Force removal (required for actual deletion of files)
g.mremove€b€Remove base raster maps
g.mremove€type€Data type(s)
g.mremove€pattern€Map name search pattern
g.mremove€exclude€Map name exclusion pattern (default: none)
Traceback (most recent call last):
   File "print_module_parameters_csv.py", line 23, in <module>
     main()
   File "print_module_parameters_csv.py", line 12, in main
     task = gtask.grassTask(cmd)
   File "/home/neteler/software/grass71/dist.x86_64-unknown-linux-
gnu/etc/python/grass/script/task.py", line 70, in __init__
     self.errorMsg = e.value
AttributeError: 'ScriptError' object has no attribute 'value'
}}}

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by martinl):

Replying to [comment:4 neteler]:

{{{
> File "print_module_parameters_csv.py", line 12, in main
> task = gtask.grassTask(cmd)
> File "/home/neteler/software/grass71/dist.x86_64-unknown-linux-
gnu/etc/python/grass/script/task.py", line 70, in __init__
> self.errorMsg = e.value
> AttributeError: 'ScriptError' object has no attribute 'value'
}}}

ah, it's something related to Python Scripting Library. Moreover it's
relevant only to trunk! In relbr70 `g.parser` is skipped properly (could
you test it? - no idea where is the difference).

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by martinl):

Replying to [comment:5 martinl]:

> ah, it's something related to Python Scripting Library. Moreover it's
relevant only to trunk! In relbr70 `g.parser` is skipped properly (could
you test it? - no idea where is the difference).

ah ok, trunk uses !ScriptError from PyGRASS
source:grass/trunk/lib/python/script/core.py#L36, it was introduced in
r61187 by zarch. In relbr70 is used own implementation of `ScriptError`
source:grass/branches/releasebranch_7_0/lib/python/script/core.py#L70.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:6&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by martinl):

Replying to [comment:6 martinl]:
> ah ok, trunk uses !ScriptError from PyGRASS
source:grass/trunk/lib/python/script/core.py#L36, it was introduced in
r61187 by zarch. In relbr70 is used own implementation of `ScriptError`
source:grass/branches/releasebranch_7_0/lib/python/script/core.py#L70.

Reported as #2410.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:7&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by neteler):

Martin, since it works for you, can you please attach the CSV file
generated with the modified script here to the ticket? Then we can go
through tomorrow at the code sprint.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:8&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by martinl):

Replying to [comment:8 neteler]:
> Martin, since it works for you, can you please attach the CSV file
generated with the modified script here to the ticket? Then we can go
through tomorrow at the code sprint.

it works just in `relbr70` not in trunk (see #2410).

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:9&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by martinl):

Replying to [comment:9 martinl]:
> Replying to [comment:8 neteler]:
> > Martin, since it works for you, can you please attach the CSV file
generated with the modified script here to the ticket? Then we can go
through tomorrow at the code sprint.
>
> it works just in `relbr70` not in trunk (see #2410).

hm, `ScriptError` should be really consolidated in `relbr70` and `trunk`,
script modified in r61946 and result attached
attachment:module_params_overview.csv

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:10&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by martinl):

Replying to [comment:10 martinl]:

> hm, `ScriptError` should be really consolidated in `relbr70` and
`trunk`, script modified in r61946 and result attached
attachment:module_params_overview.csv

reverted (r61977) since the underlaying problem has been fixed in r61959.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:11&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by wenzeslaus):

`r.texture` still has the option named `prefix`, not `basename` (#2136):
  * http://grass.osgeo.org/grass64/manuals/r.texture.html
  * http://grass.osgeo.org/grass70/manuals/r.texture.html
  * http://grass.osgeo.org/grass71/manuals/r.texture.html
Also the separator of basename and suffix should be resolved (I suppose
that it is a dot now, needs resolving of discussion in #2136).

Alternative is to use the approach taken in G7:r.neighbors where no
`basename`/`prefix` is used. Instead the standard `output` is used with
`multiple: yes` which simplifies the things in the sense that names must
be explicitly provided by user. This approach is easier for programmers
using the module but might be slightly more challenging for users running
the module manually (but it might be more understandable too since it is
clear what is the output).

''If you think this should be a separate ticket, please copy, paste and
create.''

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:12&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by wenzeslaus):

Similarly, G7:r.ros uses `output` which is in fact used as `basename`:
{{{
output=name [required]
     Prefix for output raster maps (.base, .max, .maxdir, .spotdist)
}}}

G7:r.spread is not even taking advantage of that, so the change should be
localized to `r.ros`:
{{{
max=string [required]
     Raster map containing maximal ROS (cm/min)
dir=string [required]
     Raster map containing directions of maximal ROS (degree)
base=string [required]
     Raster map containing base ROS (cm/min)
spot_dist=string
     Raster map containing maximal spotting distance (m, required with -s)
}}}

The fix in this case is: use different options for each map. What about
the names? `base_ros` or `ros_base`? Or no `ros` since the module name is
already `r.ros`? But what about `r.spread`, should this be the same? Maybe
we can use what is used in `r.spread` to make just few changes.

As [http://osgeo-org.1560.x6.nabble.com/Fwd-QGIS-Processing-amp-GRASS-
td5155460.html noted by MarkusN] QGIS Processing has problems with the
current solution:
> r.ros (maybe others): the output in this GRASS module is just a
> prefix for 3/4 output maps with an hardcoded name. Processing does not
> actually support such case.

I hope that the multiple output used by many (e.g. `r.neighbors`) and
suggested for `r.texture` is actually helps QGIS Processing (and others).
Explicit options are of course the safest way.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:13&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by wenzeslaus):

Thinking about that more, `r.texture` and actually also `r.neighbors` can
have just number of options for single raster map which would be named
`average`, `sum`, ... This completely avoids the need for multiple
input/output or basename. This approach also removes the need for option
specifying what needs to be computed (BTW, if applied to `r.ros` we can
remove the `-s` flag).

I guess that `r.neighbors` is quite challenging for QGIS Processing and
other wrappers since the output type for each map can be different and
there is [http://grass.osgeo.org/grass71/manuals/r.neighbors.html
#propagation-of-output-precision a table to say how precision is
propagated]. This might be improved a bit when separate options are used
(e.g. `average` is always `float`, `diversity` always `integer`) but
certainly not solved for all and I'm afraid that `FCELL`/`DCELL` makes the
things even more complicated.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:14&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by neteler):

Using attached table, I have harmonized a series of messages in trunk
(r62007)
and relbr7 (r62008).

Attached tables should be replaced with fresh versions.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:15&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by wenzeslaus):

The [http://grass.osgeo.org/programming7/ main page of programming
manual], section ''Vector modules and their parameters/flags'' says:

{{{
-t create new table, default
-u don't create new table
}}}

Does anybody have an idea if these two are used, kept, or some other
standard is in place?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:16&gt;
GRASS GIS <http://grass.osgeo.org>

#2409: last call for options keys consolidation
----------------------------------+-----------------------------------------
Reporter: martinl | Owner: grass-dev@…
     Type: task | Status: new
Priority: blocker | Milestone: 7.0.0
Component: Default | Version: unspecified
Keywords: standardized options | Platform: Unspecified
      Cpu: Unspecified |
----------------------------------+-----------------------------------------

Comment(by wenzeslaus):

#2136 is solved but the new approach was not applied everywhere yet. I
went through the following list (from comment:5:ticket:2136) for
standardization of basename (formerly prefix) options and changed things
from `input_prefix` and `input_prefix` to `input` and `output` while using
the standard options for base name (patch attached). I also updated the
documentation and made some fixes related to earlier changes in
prefix->basename transition.

{{{
     g.extension => prefix: Prefix where to install extension (ignored when
flag -s is given)
     i.cca => output: Output raster map prefix name
     i.landsat.acca => input_prefix: Example: 'B.' for B.1, B.2, ...
     i.landsat.toar => input_prefix: Example: 'B.' for B.1, B.2, ...
     i.landsat.toar => output_prefix: Example: 'B.toar.' generates
B.toar.1, B.toar.2, ...
     i.pansharpen => output_prefix: Prefix for output raster maps
     i.pca => output_prefix: A numerical suffix will be added for each
component map
     i.tasscap => output_prefix: Prefix for output raster maps
     i.topo.corr => output: Name (flag -i) or prefix for output raster maps
     m.nviz.script => name: Prefix of output images (default = NVIZ)
     r.blend => output_prefix: Prefix for red, green and blue output raster
maps containing
     r.rgb => output_prefix: Prefix for output raster maps (default: input)
     r.ros => output: Prefix for output raster maps (.base, .max, .maxdir,
.spotdist)
     r.texture => prefix: Prefix for output raster map(s)
     v.out.postgis => dsn: Starts with 'PG' prefix, eg. 'PG:dbname=grass'
     v.rast.stats => column_prefix: Column prefix for new attribute columns
}}}

It seems to me that `input` and `output` should be used instead of other
options (i.e. including `basename` in the option name). This also mean
that I think that the standard option definition should be changed
accordingly.

There are still some unresolved issues. As noted on [http://osgeo-
org.1560.x6.nabble.com/Fwd-QGIS-Processing-amp-GRASS-td5155460.html
mailing list] and in comment:13, there are issues with base names in QGIS
processing. Basically the issue is that things are not explicit and
actually being explicit might be better for GRASS itself regardless what
QGIS can handle or not.

It seems to me that base names on input should be avoided most of the
time. Imagery modules can use imagery groups, other modules can use
explicit list. There is a lot of modules which use one or the other.

With outputs, the situation is more complicated. I can distinguish four
different cases.

Case one, the module produces just few maps and there is not many reasons
to not provide them as separate output options. Example is G7:r.rgb
suffixing `.r`, `.g` and `.b` versus with red, green and blue options (the
change is in the diff). Another example of this is G7:r.ros which outputs
base, max and dir ROS and optionally spotting distance. Now they are all
specified using basename but G7:r.spread accepts them as separate maps
anyway and there needs to be `-s` to activate spotting (while with
separate options, it is enough just to check if option was provided). This
option is OK for scripting, it is not much work to fill manually and both
QGIS and wxGUI can handle it well (just using the standard means).

Example of case two is G7:r.neighbors, the module can generate several
statistics and you have to provide as many names for option output as is
the number of requested statistics. This might be challenging when done
manually since you must match the order but is is fine for scripting, QGIS
and wxGUI.

Example of case three is G7:r.texture where just base name is provided and
module itself adds suffixes according to selected methods. When used
manually, it is quite convenient because you don't have to care about the
names, you just have to find them afterwards. The issue for scripting is
that you don't have any idea what the suffixes are and they are indeed
different than the option values. QGIS and wxGUI which don't know the
details about the called module are completely lost. This case can be
avoided using the approach taken by G7:r.neighbors.

The last case are modules which produce (large) number of numbered maps,
these are the reason why standard base name option was created. With these
there is no other choice than to use base name. The hope with these is
that the output can be registered to temporal dataset or group. If this is
not the right choice then there is not much hope left for QGIS and wxGUI.
When scripting, you can just rely on having unique enough base name. It
should work when solving things manually because you can always check the
result and fine tune the listing.

I hope I will be able to do the necessary changes but I need to know where
it makes sense to add imagery group (as output or input) to `i.*` modules
and I need to hear opinions on moving from G7:r.texture approach to
G7:r.neighbors approach.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:17&gt;
GRASS GIS <http://grass.osgeo.org>

On Mon, Nov 17, 2014 at 3:26 AM, GRASS GIS <trac@osgeo.org> wrote:
...

As noted on [http://osgeo-org.1560.x6.nabble.com/Fwd-QGIS-Processing-amp-GRASS-td5155460.html
mailing list]

Sorry for nit-picking but it is better to refer to the osgeo list
server as nabble tends to change their server URLs sometimes, leading
to broken links. This happened already twice and they didn't bother to
add redirects.

thanks,
Markus

(Side effect: better search engine ranking for osgeo)

On Mon, Nov 17, 2014 at 10:49 AM, Markus Neteler <neteler@osgeo.org> wrote:

On Mon, Nov 17, 2014 at 3:26 AM, GRASS GIS <trac@osgeo.org> wrote:
...
> As noted on [
http://osgeo-org.1560.x6.nabble.com/Fwd-QGIS-Processing-amp-GRASS-td5155460.html
> mailing list]

Sorry for nit-picking but it is better to refer to the osgeo list
server as nabble tends to change their server URLs sometimes, leading
to broken links. This happened already twice and they didn't bother to
add redirects.

I agree. I usually try to include both. Unfortunately, my experience is

that it is hard to convince Google search to give good results for osgeo.org,
so I must often find it manually according to date.

Here is the right link:

http://lists.osgeo.org/pipermail/grass-dev/2014-August/070342.html

thanks,
Markus

(Side effect: better search engine ranking for osgeo)
_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev