[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 |
----------------------------------+-----------------------------------------

Comment(by martinl):

Replying to [comment:140 hcho]:
> BTW, we don't rename `oldvector` and `asciivector` to `old_vector` and
`ascii_vector` for the same reason? They are also two words.

make sense to me, done in r63624 and backported to relbr70 as r63625.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:141&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):

r63624 completed in r63626; r63625 completed in r63627.

BTW, there are still in lib/python/pygrass/gis/__init__.py, line 27:

{{{
          'region3d': libgis.G_ELEMENT_REGION3D,
...
          'view3d': libgis.G_ELEMENT_3DVIEW}
}}}

and

{{{
./lib/manage/option.c: else if (strcmp(p->key, "region") == 0 ||
strcmp(p->key, "region3d") == 0)

./gui/wxpython/gui_core/gselect.py:
'windows3d':'region3d',
./gui/wxpython/gui_core/gselect.py:
'region3d':'region3d',
./gui/wxpython/gui_core/gselect.py: 'region3D
definition':'region3d',
./gui/wxpython/gui_core/gselect.py: 'region3D
definition files':'region3d',
}}}

I suppose that "view3d" can be removed and "region3d" --> "region_3d"?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:142&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:142 neteler]:

> I suppose that "view3d" can be removed and "region3d" --> "region_3d"?

AFAIS we don't need `region3d`? Region settings contains 3D info by
default.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:143&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):

Many more updates related to the new element name changes, I tried to
catch all:
  * r63628 + r63629 update g.region/g.rename calls to use new element names
  * r63630 + r63631 update g.region/g.rename calls to use new element names

While I was at it:
  * r63634 + r63635: some shell test scripts updated

Now eagerly waiting for the nightly run of the testsuite at
http://fatra.cnr.ncsu.edu/grassgistests/summary_report/index.html

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:144&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 glynn):

Replying to [comment:137 hcho]:

> > No, not the last word of the option name. You cannot skip any words
separated by an underscore. rm= is not ambiguous in this case because the
shortest option name that matches rm= is raster_map=. If one wants
raster_map2=, use rmap2=, not rm2= (no abbreviation inside a word).
>
> I mean the shortest option name with no more following underscores. 'r'
for raster, 'm' for map, and no more underscores after 'raster_map'.

This still has a couple of problems:

1. match_option() now needs to return the "strength" of a match, and the
code which calls it (two places: once for option names, once for
opt->options values) needs to consider this when determining whether a
match is ambiguous.

2. match_option() now has to try to find a stronger match if there is one.
E.g given an option named "ab_b" and an argument "ab", the "b" can match
the second letter of the first word or the first letter of the second
word. Currently, it will find the first case and return. The change would
require that it finds the first, but keeps searching for a stronger match
in case an ambiguity subsequently needs to be resolved.

The "prefix" rule suggested in [comment:101] also requires complicating
the calling code, although it doesn't require changes to match_option()
itself. Specifically, it needs to record all of the options which matched;
it can't be reduced to "finite" state.

But keeping track of which options matched would allow us to provide a
more informative error message in the event that the given name is
ambiguous (i.e. the error message could list all of the matches).

The "strength" approach is perhaps a bit simpler to implement for the
caller, in that it doesn't need to track which options matched, only the
count and strength. If an option provides a strong match, and all prior
matches are weak, it sets the match counter to one (discarding any number
of weak matches). Once at least one strong match has been found, any weak
matches will be ignored. At the end, a match is unambiguous if the count
is one (i.e. one weak match and no strong matches, or one strong match and
any number of weak matches).

OTOH, it's also a bit less convenient, as the user has to provide some
part of each word, even when this doesn't reduce the set of candidates.
It's also possibly less clear to the user exactly what they should type to
avoid the ambiguity.

To be honest, the last point is my objection. I'd be less concerned about
the increased complexity of match_option() if the result was an
unambiguously better user experience (although I am slightly concerned
about the possibility that match_option_1() might go from having
exponential complexity in theory to actually having exponential complexity
in practice).

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:145&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):

A question related to the match_option. It concerns the change with
potentially most impact in the next weeks regarding the users:

{{{
g.region rast=rastermap -p
ERROR: g.region: Sorry, <rast=> is ambiguous

# as it should now be
# g.region raster=rastermap -p
}}}

It would be very helpful to advertise the candidates here, like:
{{{
g.region rast=rastermap -p
ERROR: g.region: Sorry, <rast=> is ambiguous. Choices: <raster>,
<raster_3d>
}}}

Could that be implemented?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:146&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:143 martinl]:
> Replying to [comment:142 neteler]:
>
> > I suppose that "view3d" can be removed and "region3d" --> "region_3d"?
>
> AFAIS we don't need `region3d`? Region settings contains 3D info by
default.

since `G_ELEMENT_3DVIEW` and `G_ELEMENT_REGION3D` are not used in the code
I took liberty to remove them in r63639. If no objection I will do
backport to relbr70.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:147&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 [comment:147 martinl]:
> Replying to [comment:143 martinl]:
> > Replying to [comment:142 neteler]:
> >
> > > I suppose that "view3d" can be removed and "region3d" -->
"region_3d"?
> >
> > AFAIS we don't need `region3d`? Region settings contains 3D info by
default.
>
> since `G_ELEMENT_3DVIEW` and `G_ELEMENT_REGION3D` are not used in the
code I took liberty to remove them in r63639.

But also the code indicated in comment:142 should be cleaned at the same
time.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:148&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:148 neteler]:
> > since `G_ELEMENT_3DVIEW` and `G_ELEMENT_REGION3D` are not used in the
code I took liberty to remove them in r63639.
>
> But also the code indicated in comment:142 should be cleaned at the same
time.

sure, done in r63642.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:149&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 [comment:144 neteler]:
> Many more updates related to the new element name changes, I tried to
catch all:

  * still found more, especially in the test scripts. Fixed in r63644 +
r63649.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:150&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):

Updated temporal: renamed parameters extrdir/workdir -> directory for
consistency (as earlier done for other t.* modules) in r63650 + r63651

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:151&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):

For those who would like to help with our effort, please check
attachment:module_params_overview-2014-12-12.ods and inform us about
remaining parameter inconsistencies. Any renaming should be done before we
release RC1 (planned for 29/12)/

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:152&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):

Any objections to rename

{{{
bgcolor
}}}

to

{{{
bg_color
}}}

?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:153&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:152 martinl]:
> For those who would like to help with our effort, please check
attachment:module_params_overview-2014-12-12.ods and inform us about
remaining parameter

sorry the correct file is attachment:module_params_overview-2014-12-22.ods
!!!

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:154&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:147 martinl]:

> since `G_ELEMENT_3DVIEW` and `G_ELEMENT_REGION3D` are not used in the
code I took liberty to remove them in r63639. If no objection I will do
backport to relbr70.

done in r63684.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:155&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 [comment:153 martinl]:
> Any objections to rename
>
> {{{
> bgcolor
> }}}
>
> to
>
> {{{
> bg_color
> }}}
>
> ?

I don't see an advantage in this case. Wouldn't we have to change many
similar cases then, too?

And it would really become back_ground_color (could then be abbreviate as
bg_color) which is way too long.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:156&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 hellik):

Replying to [comment:154 martinl]:
> Replying to [comment:152 martinl]:
> > For those who would like to help with our effort, please check
attachment:module_params_overview-2014-12-12.ods and inform us about
remaining parameter
>
> sorry the correct file is
attachment:module_params_overview-2014-12-22.ods !!!

for a better overview I've added a pivot analysis with options/flags,
their description and the count to the sheet.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:157&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 hellik):

Replying to [comment:154 martinl]:
> Replying to [comment:152 martinl]:
> > For those who would like to help with our effort, please check
attachment:module_params_overview-2014-12-12.ods and inform us about
remaining parameter
>
> sorry the correct file is
attachment:module_params_overview-2014-12-22.ods !!!

maybe some candidates?

{{{
stream_rast Name for output raster map with unique stream ids
stream_vect Name for output vector map with unique stream ids

transmissivity_singleway Name of single-way transmissivity raster
map [0.0-1.0]
transmissivitysingleway Name of the single-way atmospheric
transmissivitymap [-]

walk_coeff Coefficients for walking energy formula parameters a,b,c,d

transport_coeff Name of transport capacity coefficient raster map [s]

topidxclass Topographic index class
topidxstats Name of topographic index statistics file

tiledimension The dimensions of the tiles used in the output raster3d
map (XxYxZ or default: 16x16x8)
tilesize The maximum tile size in kilo bytes. Default is 32KB.

swatershed Name for output sink-watershed raster map

start_rast Name of starting raster points map

stab Set the flow stabilizing scheme (full or exponential upwinding).
staend The user defined values that indicate start, intermediate and end
status in the indicator output space time raster dataset

sourceproj Source projection
sourcescale Conversion factor from units to meters in source
projection

soilheatflux Name of instantaneous soil heat flux raster map [W/m2]
                                 Name of soil heat flux raster map [W/m2]
soilmoisture Name for output root zone soil moisture raster map

slope_tol Slope tolerance that defines a 'flat' surface (degrees)

segmax Maximum length of segment on network
                 Maximum number of points in a segment

savehistory Text file in which to save history
savesettings Name for output file where to save current settings

rgbmaps Three (R,G,B) 3D raster maps to create RGB values
[redmap,greenmap,bluemap]
                 Three (r,g,b) raster maps to create RGB values
[redmap,greenmap,bluemap]

productname Name of MODIS product type

pclay Name of soil clay fraction raster map [0.0-1.0]

pcurvature Name for output profile curvature map (or fxx)
                 Name for output profile curvature raster map

outtopidxstats Name for output topographic index statistics file

nwalkers Number of walkers
                         Number of walkers, default is twice the number of
cells

num_partitions Read the input files in this number of chunks

npartitions Number of partitions

ncolumn Node cost column
                 Node cost column (number)

min_area Minimum size of area to be imported (square units)
min_size Minimum number of pixels in a class
min_slope Minimum slope value (in percent) for which aspect is
computed

mcurvature Name for output mean curvature 3D raster map
                         Name for output mean curvature map (or fxy)
                         Name for output mean curvature raster map

loadhistory Text file from which to load history
loadsettings Name of input file to read settings from
localutctime Name of time of satellite overpass raster map [local time
in UTC]

insol_time Output insolation time raster map [h] (mode 2 only)

infil Name of runoff infiltration rate raster map [mm/hr]
infil_value Runoff infiltration rate unique value [mm/hr]
infile Input file with one input raster map name and data point position
per line, field separator between name and sample point is |
init_time Initial time for current simulation (0) (min)

gcurvature Name for output Gaussian curvature 3D raster map

beam_rad Output beam irradiance [W.m-2] (mode 1) or irradiation
raster map [Wh.m-2.day-1] (mode 2)

afcolumn Arc forward/both direction(s) cost column (number)

acolumn Arcs' cost column (for both directions)

abcolumn Arc backward direction cost column (number)
                         EXPERIMENTAL: Arc backward direction cost column
(number)
}}}

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:158&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):

It seems to me in case of `d.shade` (and perhaps few other modules) the
options have unnecessary "map" in the name. It might be enough just to
leave out the "map" unless it is there to distinguish from other option.

`d.shade` can change from

{{{
d.shade drapemap=elevation reliefmap=slope
}}}

to

{{{
d.shade drape=elevation relief=slope
}}}

It seems to me that the "map" in the name does not add much information
(that it is clear that input is some map or raster).

However, in case of `d.shade`, I'm actually suggesting:

{{{
d.shade color=elevation shade=slope
}}}

which more describes the meaning of the input rasters for me.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2409#comment:159&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:159 wenzeslaus]:
> It seems to me in case of `d.shade` (and perhaps few other modules) the
options have unnecessary "map" in the name. It might be enough just to
leave out the "map" unless it is there to distinguish from other option.

I would agree with you, please feel free to apply this changes.

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