[GRASS-dev] [GRASS GIS] #1031: More specific parameter information in command line help

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
     Type: enhancement | Status: new
Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Keywords: parser, help | Platform: All
      Cpu: All |
--------------------------+-------------------------------------------------
I would like to suggest the implementation of a more informative command
line help for grass modules. Additionally to the current parameters the
status(age), type and if it is required or not should be displayed.

A patch for parser_help.c is attached.

Example r.buffer:

{{{
lib/gis>r.buffer help

Description:
  Creates a raster map layer showing buffer zones surrounding cells that
contain non-NULL category values.

Keywords:
  raster, buffer

Usage:
  r.buffer [-z] input=name output=name distances=value[,value,...]
    [units=string] [--overwrite] [--verbose] [--quiet]

Flags:
   -z Ignore zero (0) data cells instead of NULL cells
  --o Allow output files to overwrite existing files
  --v Verbose module output
  --q Quiet module output

Parameters:
       input Name of input raster map
      output Name for output raster map
   distances Distance zone(s)
       units Units of distance
               options: meters,kilometers,feet,miles,nautmiles
               default: meters
}}}

Will change into

{{{
lib/gis> r.buffer help

Description:
  Creates a raster map layer showing buffer zones surrounding cells that
contain non-NULL category values.

Keywords:
  raster, buffer

Usage:
  r.buffer [-z] input=name output=name distances=value[,value,...]
    [units=string] [--overwrite] [--verbose] [--quiet]

Flags:
   -z Ignore zero (0) data cells instead of NULL cells
  --o Allow output files to overwrite existing files
  --v Verbose module output
  --q Quiet module output

Parameters:
       input Name of input raster map
               status: input
               type: cell
               required: yes
      output Name for output raster map
               status: output
               type: cell
               required: yes
   distances Distance zone(s)
               type: float
               required: yes
       units Units of distance
               options: meters,kilometers,feet,miles,nautmiles
               default: meters
               type: string
               required: no
}}}

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by hamish):

most of what you want is already given by usage:

{{{
Usage:
  r.buffer [-z] input=name output=name distances=value[,value,...]
    [units=string] [--overwrite] [--verbose] [--quiet]
}}}

things in [square] brackets are optional (according to age-old unix
convention (and prob ms-dos too))

the 'name' or 'value' on the right side of the equals sign, if not given a
custom setting by the programmer with opt->key_desc, could perhaps be
improved. see the tcltk module guis which are quite informative. I think
we still have some work to do on this for the wx module guis (see also
#886 for that).

Hamish

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by huhabla):

I am aware that some of informations i requests are given by the usage
string. But IMHO it is more convenient and easier to understand
(especially for command line beginner) to make these informations also
available in the help description. My enhancement request is only related
to the command line. It does not touch the automated gui generation.

The usage string does not always provide the convenient information if the
parameter is of type raster, vector, volume, integer or float. In many
modules input and output parameter are named related to the modules
purpose, the information if its an input or output is only present if you
read the help page.

Martin Landa currently renamed in several modules the input and output
parameter to make clear which of them are inputs or outputs. I.e:
elevation -> elevation_input, direction -> direction_output, but this is
IMHO not a good idea. It is redundant information, especially if you are
using the gui.
In my opinion there is no need to rename the parameters, we can provide
the information which the gui uses to make the data handling easy in the
command line too.

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by martinl):

Replying to [comment:2 huhabla]:
> I am aware that some of informations i requests are given by the usage
string. But IMHO it is more convenient and easier to understand
(especially for command line beginner) to make these informations also
available in the help description. My enhancement request is only related
to the command line. It does not touch the automated gui generation.

I think that the beginners usually do not use CLI at all. At least 'type'
and 'required' are redundant info

{{{
       input Name of input raster map
               status: input
               type: cell
               required: yes
}}}

We could use {{{key_desc}}} attribute for defining prompt type.

{{{
input=raster
}}}

instead of

{{{
input=name
}}}

The question is how to handle 'status' of the options.

> The usage string does not always provide the convenient information if
the parameter is of type raster, vector, volume, integer or float. In many
modules input and output parameter are named related to the modules
purpose, the information if its an input or output is only present if you
read the help page.

Right, it could be improved by better {{{key_desc}}} usage.

> Martin Landa currently renamed in several modules the input and output
parameter to make clear which of them are inputs or outputs. I.e:
elevation -> elevation_input, direction -> direction_output, but this is
IMHO not a good idea. It is redundant information, especially if you are
using the gui.

Right, see [wiki:Grass7/NewFeatures#Renamedoptions Rename options]. Your
idea about 'status' info seems to be better solution. If you prefer I can
remove '_input/optput' from paramaters key string.

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by huhabla):

Replying to [comment:3 martinl]:
> Replying to [comment:2 huhabla]:
> > I am aware that some of informations i requests are given by the usage
string. But IMHO it is more convenient and easier to understand
(especially for command line beginner) to make these informations also
available in the help description. My enhancement request is only related
to the command line. It does not touch the automated gui generation.
>
> I think that the beginners usually do not use CLI at all. At least
'type' and 'required' are redundant info
>

> {{{
> input Name of input raster map
> status: input
> type: cell
> required: yes
> }}}
>
> We could use {{{key_desc}}} attribute for defining prompt type.
>
> {{{
> input=raster
> }}}
>
> instead of
>
> {{{
> input=name
> }}}
>
> The question is how to handle 'status' of the options.
>
> > The usage string does not always provide the convenient information if
the parameter is of type raster, vector, volume, integer or float. In many
modules input and output parameter are named related to the modules
purpose, the information if its an input or output is only present if you
read the help page.
>
> Right, it could be improved by better {{{key_desc}}} usage.

Thats a good idea. We could start modifying the default options in
lib/gis/parser.c adding {{{key_desc}}}. Although this information is
already specified in the {{{gisprompt}}} string ... we can use the
convenient naming scheme used in {{{gisprompt}}} for {{{key_desc}}}.

>
> > Martin Landa currently renamed in several modules the input and output
parameter to make clear which of them are inputs or outputs. I.e:
elevation -> elevation_input, direction -> direction_output, but this is
IMHO not a good idea. It is redundant information, especially if you are
using the gui.
>
> Right, see [wiki:Grass7/NewFeatures#Renamedoptions Rename options]. Your
idea about 'status' info seems to be better solution. If you prefer I can
remove '_input/optput' from paramaters key string.

Well this is only my humble opinion. In case other developers prefer the
your naming schema i will accept it. Thats why i would like to discuss
this topic.

I would personally prefer to remove '_input/optput' from paramaters key
string and establish a combination of {{{key_desc}}} and the additional
"status" parameter in the command line. Maybe we can use a better name
than "status" ... like "type"? Additionally we can hide the status
parameter in case "input", "map" and "output" are used as {{{key}}}
strings?

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by huhabla):

I have attached a patch for lib/gis/parser_help.c and
lib/gis/parser_stanndard_options.c which implements the discussed ideas.

Examples:

{{{
lib/gis> r.neighbors help

Description:
  Makes each cell category value a function of the category values assigned
to the cells around it, and stores new cell values in an output raster map
layer.

Keywords:
  raster

Usage:
  r.neighbors [-ac] input=raster [selection=raster] output=raster
    [method=string] [size=value] [title=phrase] [weight=filename]
    [gauss=value] [quantile=value] [--overwrite] [--verbose] [--quiet]

Flags:
   -a Do not align output with the input
   -c Use circular neighborhood
  --o Allow output files to overwrite existing files
  --v Verbose module output
  --q Quiet module output

Parameters:
       input Name of input raster map
   selection Name of an input raster map to select the cells which should
be processed
               type: input
      output Name for output raster map
      method Neighborhood operation
               options: average,median,mode,minimum,maximum,stddev,sum,
variance,diversity,interspersion,quart1,quart3,perc90,
                        quantile
               default: average
        size Neighborhood size
               default: 3
       title Title of the output raster map
      weight Text file containing weights
       gauss Sigma (in cells) for Gaussian filter
    quantile Quantile to calculate for method=quantile
               options: 0.0-1.0
               default: 0.5
}}}

{{{
lib/gis> r.buffer2 help

Description:
  Creates a raster map layer showing buffer zones surrounding cells that
contain non-NULL category values.

Keywords:
  raster, buffer

Usage:
  r.buffer2 [-z] input=raster output=raster distances=value[,value,...]
    [units=string] [--overwrite] [--verbose] [--quiet]

Flags:
   -z Ignore zero (0) data cells instead of NULL cells
  --o Allow output files to overwrite existing files
  --v Verbose module output
  --q Quiet module output

Parameters:
       input Name of input raster map
      output Name for output raster map
   distances Distance zone(s)
       units Units of distance
               options: meters,kilometers,feet,miles,nautmiles
               default: meters
}}}

{{{
lib/gis> r.gwflow help

Description:
  Numerical calculation program for transient, confined and unconfined
groundwater flow in two dimensions.

Keywords:
  raster, groundwater flow

Usage:
  r.gwflow [-f] phead=raster status=raster hc_x=raster hc_y=raster
    [q=raster] s=raster [recharge=raster] top=raster bottom=raster
    output=raster [vx=raster] [vy=raster] [budget=raster] type=string
    [river_bed=raster] [river_head=raster] [river_leak=raster]
    [drain_bed=raster] [drain_leak=raster] dt=value [maxit=value]
    [error=value] [solver=name] [--overwrite] [--verbose] [--quiet]

Flags:
   -f Allocate a full quadratic linear equation system, default is a
sparse linear equation system.
  --o Allow output files to overwrite existing files
  --v Verbose module output
  --q Quiet module output

Parameters:
        phead The initial piezometric head in [m]
                type: input
       status Boundary condition status, 0-inactive, 1-active,
2-dirichlet
                type: input
         hc_x X-part of the hydraulic conductivity tensor in [m/s]
                type: input
         hc_y Y-part of the hydraulic conductivity tensor in [m/s]
                type: input
            q Raster map water sources and sinks in [m^3/s]
                type: input
            s Specific yield in [1/m]
                type: input
     recharge Recharge map e.g: 6*10^-9 per cell in [m^3/s*m^2]
                type: input
          top Top surface of the aquifer in [m]
                type: input
       bottom Bottom surface of the aquifer in [m]
                type: input
       output The map storing the numerical result [m]
           vx Calculate and store the groundwater filter velocity vector
part in x direction [m/s]
                type: output
           vy Calculate and store the groundwater filter velocity vector
part in y direction [m/s]
                type: output
       budget Store the groundwater budget for each cell [m^3/s]
                type: output
         type The type of groundwater flow
                options: confined,unconfined
                default: confined
    river_bed The height of the river bed in [m]
                type: input
   river_head Water level (head) of the river with leakage connection in
[m]
                type: input
   river_leak The leakage coefficient of the river bed in [1/s].
                type: input
    drain_bed The height of the drainage bed in [m]
                type: input
   drain_leak The leakage coefficient of the drainage bed in [1/s]
                type: input
           dt The calculation time in seconds
                default: 86400
        maxit Maximum number of iteration used to solver the linear
equation system
                default: 100000
        error Error break criteria for iterative solvers (jacobi, sor, cg
or bicgstab)
                default: 0.0000000001
       solver The type of solver which should solve the symmetric linear
equation system
                options: cg,pcg,cholesky
                default: cg
}}}

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by glynn):

Replying to [comment:5 huhabla]:
> I have attached a patch for lib/gis/parser_help.c and
lib/gis/parser_stanndard_options.c which implements the discussed ideas.

1. The patch makes unnecessary formatting changes, which also contravene
the GRASS formatting conventions. In general, formatting changes should be
kept separate from other changes to make it easier to review the substance
of the changes. But in this case, the formatting changes just shouldn't be
there.

2. The updated output is unnecessarily verbose, which is a bad thing IMHO.
The "required" and "multiple" status can already be determined from the
"Usage:" section. Non-required options are listed in square brackets,
while multiple options use the "option=value,..." convention. If you
really want this format, it should be a separate option, e.g. --verbose-
help.

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by martinl):

Replying to [comment:6 glynn]:
> Replying to [comment:5 huhabla]:
> > I have attached a patch for lib/gis/parser_help.c and
lib/gis/parser_stanndard_options.c which implements the discussed ideas.
>
> 1. The patch makes unnecessary formatting changes, which also contravene
the GRASS formatting conventions. In general, formatting changes should be
kept separate from other changes to make it easier to review the substance
of the changes. But in this case, the formatting changes just shouldn't be
there.
>
> 2. The updated output is unnecessarily verbose, which is a bad thing
IMHO. The "required" and "multiple" status can already be determined from
the "Usage:" section. Non-required options are listed in square brackets,
while multiple options use the "option=value,..." convention. If you
really want this format, it should be a separate option, e.g. --verbose-
help.

Probably we should think about the optimalization of the attributes in
struct Option (1) Currently we have:

{{{
struct Option
{
     const char *key;
     int type;
     int required;
     int multiple;
     const char *options;
     const char **opts;
     const char *key_desc;
     const char *label;
     const char *description;
     const char *descriptions;
     const char **descs;
     char *answer;
     const char *def;
     char **answers;
     struct Option *next_opt;
     const char *gisprompt;
     const char *guisection;
     const char *guidependency;
     int (*checker) ();
     int count;
};
}}}

Parsed for wxGUI, e.g.
{{{
{'gisprompt': True, 'multiple': 'no', 'description': 'Name for output
vector map', 'guidependency': '', 'default': '', 'age': 'new', 'required':
'yes', 'value': 'lakes_buff', 'label': '', 'guisection': u'Required',
'key_desc': ['name'], 'values': , 'values_desc': , 'prompt': 'vector',
'wxId': [-327], 'element': 'vector', 'type': 'string', 'name': 'output'}
}}}

I would suggest:

  * if 'key_desc' is NULL then use as 'key_desc' 'prompt' value
  * do we need 'element' attribute - it could be probably replaced by
'prompt'
  * suggested 'status' info is determined from 'prompt' and 'age'
attribute.

(1)
http://download.osgeo.org/grass/grass7_progman/gislib.html#Complete_Structure_Members_Table

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Changes (by martinl):

* cc: martinl (added)

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by glynn):

Replying to [comment:7 martinl]:

> Probably we should think about the optimalization of the attributes in
struct Option

Yes please.

I've been suggesting that the "type system" should be re-worked since
roughly forever. I would have hoped that the wxGUI development would
produce some feedback, but so far that hasn't happened.

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by martinl):

Replying to [comment:9 glynn]:
> Replying to [comment:7 martinl]:
>
> > Probably we should think about the optimalization of the attributes in
struct Option
>
> Yes please.
>
> I've been suggesting that the "type system" should be re-worked since
roughly forever. I would have hoped that the wxGUI development would
produce some feedback, but so far that hasn't happened.

Can you elaborate a bit on your suggestion? From wxGUI POV it's required
to have info about

{{{
multiple widget properties
description and label widget label + tooltip
guidependency to updated related widgets (e.g. database -> table
-> column)
age widget properties (e.g. show only maps from current
maps for 'new')
required options for 'required tab'
value widget value
guisection tabs
prompt for widget type (gselect.*)
type input validation
}}}

see also #278

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by huhabla):

Replying to [comment:9 glynn]:
> Replying to [comment:7 martinl]:
>
> > Probably we should think about the optimalization of the attributes in
struct Option
>
> Yes please.
>
> I've been suggesting that the "type system" should be re-worked since
roughly forever. I would have hoped that the wxGUI development would
produce some feedback, but so far that hasn't happened.

Reworking the "type system" is a good idea. I would be happy to help. My
main focus is that most of the data processing grass modules supports the
generation of a fully machine readable description of its inputs and
outputs (raster, vector, file ... as well as literal data).
This will allow the automated generic connection of modules in graphical
work-flow builder as well as in WPS process builder.
The current approach leads in the right direction but could e improved.
E.g.: putting the age, element and prompt into the gisprompt string may be
replaced using functions like G_option_set_age(), G_option_set_element()
and so on. So the Option structure can be enhanced with additional
variables, which are accessed with library functions, so better parameter
storage schemes may be implemented and data handling is easier (no parsing
overhead)?

From the WPS describe process point of view, i need the following
variables:
{{{
key Identifier in WPS execute request
multiple maxOccurs in WPS execute request
description and label abstract and title for inputs and outputs
age Is used to distinguish between old -> input
(ComplexData, LiteralData) and new -> output (ComplexOutput,
LiteralOutput)
required minOccurs in WPS execute request
answer Default value
options Allowed values
prompt Is used to distinguish between raster, vector and
file
type LiteralData type
}}}

Additionally the following parameter would be very useful:
* The mime types of input and output files
* The specification of multiple outputs see #1033
* In case no output was defined (e.g. r.univar) the definition of the data
type printed to stdout is useful. There is currently no way to define the
literal data type of printed output ... and may never be, because this
mostly depends on flags or different options

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by martinl):

Replying to [comment:4 huhabla]:
> > > Martin Landa currently renamed in several modules the input and
output parameter to make clear which of them are inputs or outputs. I.e:
elevation -> elevation_input, direction -> direction_output, but this is
IMHO not a good idea. It is redundant information, especially if you are
using the gui.
> >
> > Right, see [wiki:Grass7/NewFeatures#Renamedoptions Rename options].
Your idea about 'status' info seems to be better solution. If you prefer I
can remove '_input/optput' from paramaters key string.
>
> Well this is only my humble opinion. In case other developers prefer the
your naming schema i will accept it. Thats why i would like to discuss
this topic.
>
> I would personally prefer to remove '_input/optput' from paramaters key
string and establish a combination of {{{key_desc}}} and the additional
"status" parameter in the command line. Maybe we can use a better name
than "status" ... like "type"? Additionally we can hide the status
parameter in case "input", "map" and "output" are used as {{{key}}}
strings?

OK, done in r41865, [wiki:Grass7/NewFeatures#Renamedoptions Rename
options] updated.

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by glynn):

Replying to [comment:10 martinl]:

> > I've been suggesting that the "type system" should be re-worked since
roughly forever. I would have hoped that the wxGUI development would
produce some feedback, but so far that hasn't happened.
>
> Can you elaborate a bit on your suggestion?

Currently, the "type" is defined by a combination of fields:

type, required, multiple, options, key_desc, gisprompt.

The type field defines a base type: string, integer or floating-point.
gis_prompt may refine the base type to a more specific type. options
replaces the base type with an enumeration type. required==NO converts the
type to a "Maybe" type. multiple=YES converts the type to a list type
(except that the GUI treats it as a set if the "options" field is set).
key_desc converts the type to a tuple type if the description contains
commas.

My preference would be for a structured type system as is commonly found
in high-level languages, where base types can be combined using
constructors.

Essentially, all of the above fields would be replaced by a single type
field, whose value would be a pointer to an arbitrarily complex structure,
with a set of functions for creating types. Off the top of my head, you
would need:

{{{
// Base types:
T_integer()
T_integer_subrange(int first, int last, int step)
T_integer_enumeration(int count, int val0, [, int val1, ...])
T_real()
T_real_subrange(double first, bool first_inclusive, double last, bool
last_inclusive)
T_string()
T_string_enumeration(count, char* val0, [, char* val1, ...])
T_string_enumeration_with_descs(count, char* val0, char *desc0, [, char*
val1, char* desc1, ...])
T_element(enum Element element, enum Age age)

// Composite types:
T_optional(Type* base)
T_tuple(int count, Type* type0 [, Type* type1, ...])
T_record(int count, char* name0, Type* type0 [, char* name1, Type* type1,
...])
T_list(Type* type, bool allow_empty, bool allow_duplicates, bool
order_matters)
T_union(int count, Type* type0, Type* type1 [, Type* type2, ...])
}}}

So e.g.:

{{{
opt->type = T_union(2,
         T_integer_subrange(1, 99, 2),
         T_string_enumeration(2, "all", "none"));
}}}

would allow the option value to be any odd integer between 1 and 99
inclusive or the strings "all" or "none".

Variadic functions could either use a prefixed count or a NULL terminator
(but the latter won't work for numbers).

You would also need dependent base types, e.g. "column name within the
database table associated with the map specified by the input= option".

When it comes to parsing, you could implement such types using a generic
mechanism using a callback function to enumerate or validate allowed
values; however, we need to think about how this interacts with the GUI.

One possibility would be:

T_dependent(Type* (*callback)(int argc, char**argv), char* opt0 [, char*
opt1, ...])

The parser would provide an option to expand an option's type given a
partial list of option values, e.g.

{{{
d.vect --expand=rgb_column map=inmap
}}}

would convert the rgb_column= option's dependent type to a fixed
enumeration of the available columns for the specified input map.

The alternative is to simply add each case as a separate base type, i.e.:

{{{
T_database_column(char* driver_opt, char* database_opt, char* table_opt)
T_map_database_column(char* map_opt)
}}}

The --interface description would simply report the information verbatim
and the GUI would be responsible for enumeration.

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by huhabla):

Replying to [comment:13 glynn]:
> Replying to [comment:10 martinl]:
>
[snip]
>
> So e.g.:
>
> {{{
> opt->type = T_union(2,
> T_integer_subrange(1, 99, 2),
> T_string_enumeration(2, "all", "none"));
> }}}
>
> would allow the option value to be any odd integer between 1 and 99
inclusive or the strings "all" or "none".

That sounds brilliant!
I have implemented a simple prototype, to get an idea how this may work.
Files are attached. Do you think my implementation points in the right
direction?

>
> Variadic functions could either use a prefixed count or a NULL
terminator (but the latter won't work for numbers).
>
> You would also need dependent base types, e.g. "column name within the
database table associated with the map specified by the input= option".

I am not sure if i understand that, What is the benefit of dependent base
type?

>
> When it comes to parsing, you could implement such types using a generic
mechanism using a callback function to enumerate or validate allowed
values; however, we need to think about how this interacts with the GUI.

The implementation i attached uses a simple callback to generically prints
the content of the type structures. The callback will be attached while
memory allocation.

{{{

struct T_type {
   enum T_TYPES type;
   void *p; /*Pointer to a type structure*/
   void (*print)(struct T_type*);
};

struct Option {
   struct T_type *key;
   struct T_type *type;
   struct T_type *description;
   struct T_type *answer;
};

struct Option* T_define_option()
{
   struct Option *opt;

   opt = (struct Option*)calloc(1, sizeof(struct Option));
   opt->key = NULL;
   opt->type = NULL;
   opt->description = NULL;
   opt->answer = NULL;

   return opt;
}

struct T_type* T_real_subrange(double first, enum T_BOOL first_inclusive,
double last, enum T_BOOL last_inclusive)
{
   struct T_type *t;
   struct T_type_real_subrange *it;

   t = T_type_alloc();

   it = (struct T_type_real_subrange*)calloc(1, sizeof(struct
T_type_real_subrange));
   it->first = first;
   it->last = last;
   it->first_inclusive = first_inclusive;
   it->last_inclusive = last_inclusive;

   t->type = REAL_SUBRANGE;
   t->p = it;
   t->print = T_real_subrange_print;

   return t;
}

void T_real_subrange_print(struct T_type* type)
{
   struct T_type_real_subrange* p;
   p = (struct T_type_real_subrange*)type->p;
   if(p) {
     fprintf(stderr, "first: %f\n", p->first);
     fprintf(stderr, "last: %f\n", p->last);
     fprintf(stderr, "first_include: %i\n", p->first_inclusive);
     fprintf(stderr, "last_include: %i\n", p->last_inclusive);
   }

   return;
}

   struct Option *opt5 = T_define_option();
   opt5->key = T_string("val");
   opt5->type = T_real_subrange(100.0, True, 200.0, True);
   opt5->description = T_string("Values in between");
   //Print the content to stdout
   opt5->key->print(opt5->key);
   opt5->description->print(opt5->description);
   opt5->type->print(opt5->type);

//This will bw the result at stdout:
val
Values in between
first: 100.000000
last: 200.000000
first_include: 1
last_include: 1

}}}

>
> One possibility would be:
>
> T_dependent(Type* (*callback)(int argc, char**argv), char* opt0 [, char*
opt1, ...])
>
> The parser would provide an option to expand an option's type given a
partial list of option values, e.g.
>
> {{{
> d.vect --expand=rgb_column map=inmap
> }}}
>
> would convert the rgb_column= option's dependent type to a fixed
enumeration of the available columns for the specified input map.

Well, sorry, you lost me at this point.
But i think i will catch up, if you could add more examples. :slight_smile:

I think your approach has enormous potential, but i am also concerned that
it may be to complicated for grass module programmers?

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

#1031: More specific parameter information in command line help
--------------------------+-------------------------------------------------
  Reporter: huhabla | Owner: grass-dev@lists.osgeo.org
      Type: enhancement | Status: new
  Priority: minor | Milestone: 7.0.0
Component: libgis | Version: svn-trunk
Resolution: | Keywords: parser, help
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Comment (by glynn):

Replying to [comment:14 huhabla]:
> I have implemented a simple prototype, to get an idea how this may work.
Files are attached. Do you think my implementation points in the right
direction?

I think that it's too early to be bothering with implementation details
right now. I'd rather clarify the concept then the interface.

> > Variadic functions could either use a prefixed count or a NULL
terminator (but the latter won't work for numbers).
> >
> > You would also need dependent base types, e.g. "column name within the
database table associated with the map specified by the input= option".
>
> I am not sure if i understand that, What is the benefit of dependent
base type?

I'm talking about the situation where the set of valid values for one
option is dependent upon the value of another option. E.g. for options
which specify the name of a database column, valid values are column names
in a particular table, typically the table associated with a specific map.

E.g. for d.vect, there are rgb_column=, wcolumn=, size_column=,
rot_column=, and attrcolumn= options; valid values for these options are
column names from the table associated with the map specified by the map=
option. Ideally, the GUI would be able to offer a list of column names
once the user has chosen a map.

IIRC, at present the GUI updates the list whenever the user chooses a map.
If a module has more than one option whose value is a vector map name, the
GUI has no way of knowing which one determines the set of valid column
names.

Similar issues exist if valid option values are categories within a raster
map, or cell values within the map's range, or coordinates within the
map's bounds, subgroups within an imagery group, etc.

> > When it comes to parsing, you could implement such types using a
generic mechanism using a callback function to enumerate or validate
allowed values; however, we need to think about how this interacts with
the GUI.

Right. This is why I'm inclined towards a "closed sum" approach, with a
finite, fixed set of base types and constructors. Having the GUI invoke
the module to execute a call-back should be a last resort. Apart from
efficiency issues, it forces the GUI to use a generic interface rather
than more specialised interfaces.

> I think your approach has enormous potential, but i am also concerned
that it may be to complicated for grass module programmers?

Most modules will only need relatively straightforward types, e.g. "raster
map name". The most common cases will just use G_define_standard_option().
Still-common but more complex cases (e.g. a database column for a
particular map) can have utility functions written for them.

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