last call for options keys consolidation

[martinl]

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.

[martinl]

modules options overview

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

[neteler]

Replying to 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.

[martinl]

Replying to 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)

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

[martinl]

Replying to 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).

[martinl]

Replying to 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​.

[martinl]

Replying to 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.

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

[martinl]

Replying to 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).

[martinl]

modules options overview (csv)

[martinl]

Replying to martinl:

Replying to 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​

[martinl]

Replying to 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.

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

[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 ​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.

[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 ​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.

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

[neteler]

Updated table (€ is field separator)

[wenzeslaus]

The ​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?

[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 ​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.

[wenzeslaus]

changing prefix defined in different ways to standard option base name, renaming to input and output and introducing red, green and blue options to r.rgb

[martinl]

related to attachment:prefix_to_basename.diff​

I don't fully understand why to define key 'output' for basename options. I thought that G_OPT_BASENAME_OUTPUT will have default key like 'basename_output' or 'output_basename' (better for backward compatibility). Similarly G_OPT_BASENAME_INPUT 'input_basename'.

[wenzeslaus]

Replying to martinl:

related to attachment:prefix_to_basename.diff​

I don't fully understand why to define key 'output' for basename options. I thought that G_OPT_BASENAME_OUTPUT will have default key like 'basename_output' or 'output_basename' (better for backward compatibility). Similarly G_OPT_BASENAME_INPUT 'input_basename'.

Currently, it seems to me that the current option naming policy is to use input or output regardless of type. So input can be raster for one module but vector or imagery group for another. Basename is just another item in the list of possible types/meanings.

I think that in case that the output is group or spatio temporal dataset, the option name basename is appropriate if you want to set the basename for maps differently from the name of the group.

When opening #2136 it was not clear to me when output, output_basename/basename_output or basename are appropriate. The motivation for #2136 was unification and creation of standard mechanism in the first place.

But especially after going through Pietro's list in comment:5:ticket:2136, it seems to me that basename is nothing special and the default/most used name for the option should be the same as if it would be raster or vector, so output and input in these cases.

[martinl]

Replying to wenzeslaus:

Currently, it seems to me that the current option naming policy is to use input or output regardless of type. So input can be raster for one module but vector or imagery group for another. Basename is just another item in the list of possible types/meanings.

then why the default key is set to basename ? source:grass/trunk/lib/gis/parser_standard_options.c#L336​

[martinl]

For those how would like to spend some energy and check possible inconsistency in parameter names I attached attachment:module_params_overview-2014-11-24.csv.gz​

[wenzeslaus]

Replying to martinl:

Replying to wenzeslaus:

Currently, it seems to me that the current option naming policy is to use input or output regardless of type. So input can be raster for one module but vector or imagery group for another. Basename is just another item in the list of possible types/meanings.

then why the default key is set to basename ? source:grass/trunk/lib/gis/parser_standard_options.c#L336​

And also the description could be better. I can fix it when I get to it.

[glynn]

Replying to martinl:

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.

It should be safe to change a key provided that the old value is an accepted abbreviation for the new value.

And on that note, if anyone is planning on changing any keys, please

1. Avoid abbreviating keys; the user can always abbreviate, but they can't "unabbreviate".

2. Place an underscore between distinct words, to increase the set of accepted abbreviations (e.g. "line_color" can be abbreviated to "lcol" bult "linecolor" can't).

To recap: if an option key consists of multiple words (sequences of characters separated by underscores), the key can be abbreviated to any combination of prefixes of the individual words. The first letter of the first word must be provided; subsequent words may be omitted entirely. Underscores are optional.

The main limitation is that each word can only be abbreviated to a (possibly empty) prefix, so

1. Compound words cannot have their components abbreviated individually, so "background" cannot be abbreviated to "bg". "back_ground" can be abbreviated to "background" or "bg", but looks ugly.

2. Plurals cannot be abbreviated to a plural, so "columns" can be abbreviated to "col" but not to "cols". Again, "column_s" could be abbreviated to "column_s" or "col" or "cols", but also looks ugly.

It would be reasonably straightforward to extend the code to support an "invisible" word separator, which would behave like underscore but would be omitted from the --help output. So if "background" was changed to e.g. "back'ground" it would show as "background" in the --help output but could be abbreviated to "bg".

One possible drawback is that if an abbreviation was rejected due to being a valid abbreviation for more than option, the conflicting option wouldn't necessarily be apparent to the user (due to the separator being hidden).

[martinl]

Replying to glynn:

Replying to martinl:

1. Compound words cannot have their components abbreviated individually, so "background" cannot be abbreviated to "bg". "back_ground" can be abbreviated to "background" or "bg", but looks ugly.

thanks for useful comments, there are a lot of options which are abbreviated. Eg.

bgcolor Background color

Do we prefer to leave as it is or change to

background_color

?

Then it can be abbreviated to

bcolor

but not to

bgcolor

That would require to rename the option to

back_ground_color

which is ugly as Glynn already noted.

[martinl]

Replying to martinl:

Do we prefer to leave as it is or change to

background_color

?

Then it can be abbreviated to

bcolor

but not to

bgcolor

The attachment:renamed_options.diff​ adds to the parser support for renamed options. Eg.

d.erase bgcolor=black

will continue to work with appropriate warning

WARNING: Please update the interface of the module: option <bgcolor> has been renamed to <background_color> in GRASS 7.0

Make sense to you?

[annakrat]

Replying to martinl:

Replying to martinl:

Do we prefer to leave as it is or change to

background_color

?

Then it can be abbreviated to

bcolor

but not to

bgcolor

In this case I would keep bgcolor, I don't think we have to unabbreviate everything just for the sake of consistency and in this case as you show, the abbrevated versions are not really suitable, unless Glynn implements the new mechanism, which I think would be confusing.

The attachment:renamed_options.diff​ adds to the parser support for renamed options. Eg.

d.erase bgcolor=black

will continue to work with appropriate warning

WARNING: Please update the interface of the module: option <bgcolor> has been renamed to <background_color> in GRASS 7.0

Make sense to you?

This is great, I haven't tested it yet, but it will be very useful.

[annakrat]

I will change r.colors options (raster -> rast, volume -> rast3d) unless someone is against, sometime soon.

[martinl]

Replying to annakrat:

In this case I would keep bgcolor, I don't think we have to unabbreviate everything just for the sake of consistency and in this case as you show, the abbrevated versions are not really suitable, unless Glynn implements the new mechanism, which I think would be confusing.

WARNING: Please update the interface of the module: option <bgcolor> has been renamed to <background_color> in GRASS 7.0

Make sense to you?

This is great, I haven't tested it yet, but it will be very useful.

If we use the patch noted above than we can happily rename bgcolor to background_color I would say...

[martinl]

Replying to annakrat:

I will change r.colors options (raster -> rast, volume -> rast3d) unless someone is against, sometime soon.

same should be done with d.rast.leg

[glynn]

Replying to annakrat:

I will change r.colors options (raster -> rast, volume -> rast3d) unless someone is against, sometime soon.

"raster" shouldn't be abbreviated. "volume" should probably be "raster_3d" (for which "raster3d" and "rast3d" are accepted abbreviations).

[glynn]

Replying to martinl:

Do we prefer to leave as it is or change to

background_color

?

Then it can be abbreviated to

bcolor

Except: d.vect currently has both bgcolor= (label background color) and bcolor= (label border colour). If these were changed to background_color and border_color, bcolor would be rejected as ambiguous.

Another possibility with regards to abbreviating compound words would be to use an upper-case letter (rather than a specific character, such as the single quote in the previous example), so backGround_color would work like back_ground_color but arguably isn't quite as ugly. Similarly, "columnS" doesn't seem as bad as column_s.

[annakrat]

Replying to glynn:

Replying to annakrat:

I will change r.colors options (raster -> rast, volume -> rast3d) unless someone is against, sometime soon.

"raster" shouldn't be abbreviated. "volume" should probably be "raster_3d" (for which "raster3d" and "rast3d" are accepted abbreviations).

So we then have to rename also option rast, rast3d in g.region I suppose, and perhaps some others. We shouldn't forget that although unabbreviating doesn't cause problems in the command line, python scripts require the full name of the command.

[annakrat]

Replying to glynn:

Another possibility with regards to abbreviating compound words would be to use an upper-case letter (rather than a specific character, such as the single quote in the previous example), so backGround_color would work like back_ground_color but arguably isn't quite as ugly. Similarly, "columnS" doesn't seem as bad as column_s.

Please don't do that. GRASS suffers from "too many ways to do the same thing", let's not make it worse.

[wenzeslaus]

Replying to annakrat:

Replying to glynn:

Replying to annakrat:

I will change r.colors options (raster -> rast, volume -> rast3d) unless someone is against, sometime soon.

"raster" shouldn't be abbreviated. "volume" should probably be "raster_3d" (for which "raster3d" and "rast3d" are accepted abbreviations).

So we then have to rename also option rast, rast3d in g.region I suppose, and perhaps some others. We shouldn't forget that although unabbreviating doesn't cause problems in the command line, python scripts require the full name of the command.

I would not unabbreviate everything. Some standard widely used abbreviations such as rast or vect are OK I would say. Non-abbreviated terms should be used for less standard things such as elevation or index. This is a good compromise between clear readable names and overly long options and module calls.

Of course in some cases unabbreviating would be quite harmless concerning total length of option(s), for example rast -> raster but I don't have an idea how badly this would work for strds (which I'm not comfortable with but I don't have a better idea).

Also sometimes you just have to make the option name shorter by leaving out some words. Of course this is combined a lot with abbreviating because it would be not readable anyway, for example npmin from G7:v.surf.rst which has meaning Minimum number of points for approximation in a segment, the only meaningful not shortened not abbreviated name I can think of is minimum_of_points_for_approximation which is juts too long.

This brings me to my final point. If we unabbreviate and unshorten everything and we will provide a lot of different options how to abbreviate it then we can naturally expect that people will use it a lot and they will of course use different abbreviations. But this, I think, defeats the purpose of unabbreviated option that is readability (and auto-documentation) because by looking at the command with abbreviated option you are not able to say what they mean.

Therefore, I think that using the abbreviations occasionally on places where everybody would like to shorten anyway is the way to go.

[wenzeslaus]

Replying to martinl:

Replying to martinl:

Do we prefer to leave as it is or change to

background_color

?

Then it can be abbreviated to

bcolor

but not to

bgcolor

The attachment:renamed_options.diff​ adds to the parser support for renamed options. Eg.

d.erase bgcolor=black

will continue to work with appropriate warning

WARNING: Please update the interface of the module: option <bgcolor> has been renamed to <background_color> in GRASS 7.0

Make sense to you?

We need something like this for a long time already. It is really good as long as it is used as you are proposing that is to maintain backwards compatibility. I wouldn't like to see that used as a way to provide different names for one option.

[martinl]

Replying to glynn:

Except: d.vect currently has both bgcolor= (label background color) and bcolor= (label border colour). If these were changed to background_color and border_color, bcolor would be rejected as ambiguous.

It has been already renamed to label_bgcolor and label_bcolor in r62945.

Another possibility with regards to abbreviating compound words would be to use an upper-case letter (rather than a specific character, such as the single quote in the previous example), so backGround_color would work like back_ground_color but arguably isn't quite as ugly. Similarly, "columnS" doesn't seem as bad as column_s.

I thought we have a rule not to use uppercase in key names. I would personally keep it.

[martinl]

Replying to annakrat:

Replying to glynn:

Another possibility with regards to abbreviating compound words would be to use an upper-case letter (rather than a specific character, such as the single quote in the previous example), so backGround_color would work like back_ground_color but arguably isn't quite as ugly. Similarly, "columnS" doesn't seem as bad as column_s.

Please don't do that. GRASS suffers from "too many ways to do the same thing", let's not make it worse.

I would agree with Anna.

[martinl]

Replying to wenzeslaus:

Replying to martinl:

related to attachment:prefix_to_basename.diff​

I don't fully understand why to define key 'output' for basename options. I thought that G_OPT_BASENAME_OUTPUT will have default key like 'basename_output' or 'output_basename' (better for backward compatibility). Similarly G_OPT_BASENAME_INPUT 'input_basename'.

Currently, it seems to me that the current option naming policy is to use input or output regardless of type. So input can be raster for one module but vector or imagery group for another. Basename is just another item in the list of possible types/meanings.

OK, done in r62966

[mlennert]

Replying to glynn:

Replying to annakrat:

I will change r.colors options (raster -> rast, volume -> rast3d) unless someone is against, sometime soon.

"raster" shouldn't be abbreviated.

+1

The problem is obviously more the inconsistency between modules than one choice over the other, but I would also plead for unabbreviated keys (unless absolutely infeasible).

Moritz

[martinl]

Replying to mlennert:

"raster" shouldn't be abbreviated.

+1

The problem is obviously more the inconsistency between modules than one choice over the other, but I would also plead for unabbreviated keys (unless absolutely infeasible).

right, this means to update element_list, see eg. inconsistency in

g.findfile -l
List of available elements:
  rast      (raster map(s))
  rast3d    (3D raster map(s))
  vect      (vector map(s))
  oldvect   (old (GRASS 5.0) vector map(s))
  asciivect (ASCII vector map(s))
  labels    (paint label file(s))
  region    (region definition(s))
  region3d  (3D region definition(s))
  group     (imagery group(s))

g.findfile element=vector

will not work...

[annakrat]

Replying to glynn:

Replying to annakrat:

I will change r.colors options (raster -> rast, volume -> rast3d) unless someone is against, sometime soon.

"raster" shouldn't be abbreviated. "volume" should probably be "raster_3d" (for which "raster3d" and "rast3d" are accepted abbreviations).

I don't like "raster_3d". If we rename "rast" to "raster", which seems reasonable, I would go with "raster3d", even though you can't shorten it.

[neteler]

Note: For testing the updated parameters, use at least r63176.

[neteler]

Remaining parameter issues after r63068

[neteler]

Replying to neteler:

Note: For testing the updated parameters, use at least r63176.

I have added a ODS table with remaining parameter issues after r63176 to be fixed.

[martinl]

Replying to annakrat:

"raster" shouldn't be abbreviated. "volume" should probably be "raster_3d" (for which "raster3d" and "rast3d" are accepted abbreviations).

I don't like "raster_3d". If we rename "rast" to "raster", which seems reasonable, I would go with "raster3d", even though you can't shorten it.

in r63189 I renamed

     rast -> raster
     rast3d -> volume
     vect -> vector
     oldvect -> oldvector
     asciivect -> asciivect

It means that current abbreviated option will still work expect of rast3d which needs to be renamed to volume.

[martinl]

Replying to martinl:

in r63189 I renamed

btw, do we need region3d ? ASAIK the region is 3D by default. Martin

[martinl]

Replying to martinl:

>      asciivect -> asciivect

I meant

       asciivect -> asciivector

[wenzeslaus]

Replying to martinl:

Replying to annakrat:

"raster" shouldn't be abbreviated. "volume" should probably be "raster_3d" (for which "raster3d" and "rast3d" are accepted abbreviations).

I don't like "raster_3d". If we rename "rast" to "raster", which seems reasonable, I would go with "raster3d", even though you can't shorten it.

in r63189 I renamed

rast3d -> volume

It means that current abbreviated option will still work expect of rast3d which needs to be renamed to volume.

Why to volume? Although there was no agreement, we were often replacing all "voxel" and "volumne" (and others) by "3D raster" so far. The only three places I know about where "volume" is used are ​v.vol.rst (just the name, not even the keyword), GUI menu/toolbox, and GUI for NVIZ.

I'm personally against "volume" because this term means too much other things. (Does volume computations refer to 3D raster/voxels/voxel models or to cubic meters of soil or water?) It even does not bring any advantages for module family name (currently r3) which would be v but v is already used.

If we are be replacing "3D raster", we should do it everywhere including manual and module family name (​Manual: Raster3D commands).

[martinl]

Replying to wenzeslaus:

It means that current abbreviated option will still work expect of rast3d which needs to be renamed to volume.

Why to volume? Although there was no agreement, we were often replacing all "voxel" and

I was discussing that with MarkusN and MarkusM and we didn't find a better solution. Element 'volume' has big advantage: shorten 'rast' will still work. Moreover 'rast3d' was somehow weird, "Volume" is used in the GUI (menu).

"volumne" (and others) by "3D raster" so far. The only three places I know about where

"Voxel" is an element of the 3D grid. I would stay in the manual with "3D raster map" or "volume map".

I'm personally against "volume" because this term means too much other things. (Does volume computations refer to 3D raster/voxels/voxel models or to cubic meters of soil or water?) It

Feel free to suggest better solution. We don't want to use shorten element names, we would prefer that shorten names will still work (g.list rast).

If we are be replacing "3D raster", we should do it everywhere including manual and module family name (​Manual: Raster3D commands).

No, I would stay with "3D raster map" or "volume map". We just changed element list name, nothing else.

[wenzeslaus]

Replying to martinl:

Replying to wenzeslaus:

It means that current abbreviated option will still work expect of rast3d which needs to be renamed to volume.

Why to volume? Although there was no agreement, we were often replacing all "voxel" and

I was discussing that with MarkusN and MarkusM and we didn't find a better solution. Element 'volume' has big advantage: shorten 'rast' will still work.

Sorry, but as you see I don't agree. Moreover, as I said before, I don't see the point in making everything longer and then thinking about how to shorten it. Well-decided short option/type name is better.

If we would use rast and rast3d there wouldn't be any need to think about shortening and thus no need to use something else than 3D raster.

Moreover 'rast3d' was somehow weird,

Not for me, I don't say it's ideal but I considered it as good enough (also for lack of other better options).

If something is weird to me, it is r3 in the beginning of module names because it has two characters not one. However, I haven't seen anything which could be used to replace that (since both volume and voxel starts with v).

"Volume" is used in the GUI (menu).

And I always wanted to change that. r3 is used in the name of the modules.

"volume" (and others) by "3D raster" so far. The only three places I know about where

"Voxel" is an element of the 3D grid.

Sure, "voxel" is "3D pixel" and since pixel is a part of 2D raster, voxel is a part of 3D raster.

I would stay in the manual with "3D raster map" or "volume map".

If we are be replacing "3D raster", we should do it everywhere including manual and module family name (​Manual: Raster3D commands).

No, I would stay with "3D raster map" or "volume map". We just changed element list name, nothing else.

That's the problem. This would only make the situation worse. Besides so far favored "3D raster map" we would start to push also "volume map" and I'm not even mentioning "voxel map" and "3D grid".

I'm personally against "volume" because this term means too much other things. (Does volume computations refer to 3D raster/voxels/voxel models or to cubic meters of soil or water?) It

Feel free to suggest better solution. We don't want to use shorten element names, we would prefer that shorten names will still work (g.list rast).

My suggestion is not to make the options longer in any cost and rather use reasonable shortcut as the only name. In case of rasters and 3D rasters it is rast and rast3d which is even backwards compatible.

If we want users to learn that integer is CELL and double precision floating point is DCELL then I believe they can also handle rast being raster and rast3d being 3D raster.

Speaking about naming of 3D raster, with my suggestion, there is no need to rename because there is no pressure for shortcuts. I would just propagate 3D raster everywhere, solve in any way few issues with 3D being at the beginning or at the end, and forgot about alternative names.

GRASS GIS is great because for rasters and vectors it is using the same terms as computer graphics which is great because it is simple and consistent. It is much better than some infamous software whose users are calling digital elevation model image, polygons class, and data Shapefile.

Unfortunately, computer graphics does not offer a clear solution for naming the thing which is the same as raster but in 3 dimensions. But what about the simplest solution, 3D raster?

Even if there would be strong reason for renaming 3D raster (rast3d) to something else, I would rather choose voxel instead of volume because voxel is at least unambiguously related to 3D (voxel can be 3D pixel and also some more complicated 3D computer graphics thing). It is true that voxel is in fact "volume pixel" but "voxel map" clearly refers to 3D while "volume map" can mean different things. I like "voxel map" in general (much more than "volume map"). And as I said before, if rename, then rename completely, so including the change of r3 to vo. But I would prefer to not go this way.

[martinl]

Replying to wenzeslaus:

Sorry, but as you see I don't agree. Moreover, as I said before, I don't see the point in making everything longer and then thinking about how to shorten it. Well-decided short option/type name is better.

shortly saying I don't see any problem to name the element as 'volume' and use "3D raster map" in the manual and as element description..

g.list --help

       type   Data type(s)
              options: raster,volume,vector,oldvector,asciivector,labels,
                       region,region3d,group,all
               raster: raster map(s)
               volume: 3D raster map(s)
               vector: vector map(s)
               oldvector: old vector map(s) (GRASS 5.0)
               asciivector: ASCII vector map(s)
               labels: paint label file(s)
               region: region definition(s)
               region3d: 3D region definition(s)
               group: imagery group(s)
               all: all types

[wenzeslaus]

Replying to martinl:

Replying to wenzeslaus:

Sorry, but as you see I don't agree. Moreover, as I said before, I don't see the point in making everything longer and then thinking about how to shorten it. Well-decided short option/type name is better.

shortly saying I don't see any problem to name the element as 'volume' and use "3D raster map" in the manual and as element description..

I do.

[martinl]

Replying to wenzeslaus:

shortly saying I don't see any problem to name the element as 'volume' and use "3D raster map" in the manual and as element description..

I do.

Anyone else has so clear idea about that?

[annakrat]

Replying to martinl:

Replying to wenzeslaus:

shortly saying I don't see any problem to name the element as 'volume' and use "3D raster map" in the manual and as element description..

To me, this sounds like a possible source of confusion.

I do.

Anyone else has so clear idea about that?

It seems that the only reason why you want 'volume' is to enable typing 'g.list rast', which isn't enough for me. If we wouldn't change rast to raster, there would be no need to change rast3d to volume. So in my view we ended up with something worse then we had before just because we started to unabbreviate everything. I thought the initial idea was to fix only apparently bad parameter names, but this is starting to have large consequences - changes in scripts, manual pages, and confusing users.

[martinl]

Replying to annakrat:

we started to unabbreviate everything. I thought the initial idea was to fix only apparently bad parameter names, but this is starting to have large consequences - changes in scripts, manual pages, and confusing users.

with lookup table old_key:new_key we can quite easily fix all scripts and manual pages. Also parser will understand old_key will continue to work. We were planned to do cleanup some years ago, this ticket has been open 3 months ago. I believe that it will has good consequences at the end. We just need to finish the job. That's all.

[wenzeslaus]

Replying to martinl:

Replying to annakrat:

we started to unabbreviate everything. I thought the initial idea was to fix only apparently bad parameter names, but this is starting to have large consequences - changes in scripts, manual pages, and confusing users.

with lookup table old_key:new_key we can quite easily fix all scripts and manual pages. Also parser will understand old_key will continue to work. We were planned to do cleanup some years ago, this ticket has been open 3 months ago. I believe that it will has good consequences at the end. We just need to finish the job. That's all.

The purpose of unabbreviated options (and cleanup) is user friendliness through manual and interface clearness. But the consequences of your decision are two different terms for one thing. This is not user friendly and thus it goes against the original motivation for the change.

[martinl]

Replying to wenzeslaus:

with lookup table old_key:new_key we can quite easily fix all scripts and manual pages. Also parser will understand old_key will continue to work. We were planned to do cleanup some years ago, this ticket has been open 3 months ago. I believe that it will has good consequences at the end. We just need to finish the job. That's all.

done in r63209 (feel free to update source:grass/trunk/lib/gis/renamed_options​ which would be very valuable to avoid large consequences you mentioned), eg.

d.rast map=dmt vallist=500-600
WARNING: Please update the interface of the module: option <vallist> has been renamed to <values>

[...]

This is not user friendly and thus it goes against the original motivation for the change.

Sorry, I don't understand. Do you think that it's more user friendly when d.rast fails with

ERROR: Sorry, <vallist> is not a valid parameter

?

[wenzeslaus]

Replying to martinl:

Replying to wenzeslaus:

with lookup table old_key:new_key we can quite easily fix all scripts and manual pages. Also parser will understand old_key will continue to work. We were planned to do cleanup some years ago, this ticket has been open 3 months ago. I believe that it will has good consequences at the end. We just need to finish the job. That's all.

done in r63209 (feel free to update source:grass/trunk/lib/gis/renamed_options​ which would be very valuable to avoid large consequences you mentioned), eg.

d.rast map=dmt vallist=500-600
WARNING: Please update the interface of the module: option <vallist> has been renamed to <values>

[...]

This is not user friendly and thus it goes against the original motivation for the change.

Sorry, I don't understand. Do you think that it's more user friendly when d.rast fails with

ERROR: Sorry, <vallist> is not a valid parameter

?

I was referring to rename of rast3d to volume. The old_key solves the backward compatibility issue but not the issue of two different names for one thing in manual which you created by introducing volume.

[martinl]

Replying to wenzeslaus:

shortly saying I don't see any problem to name the element as 'volume' and use "3D raster map" in the manual and as element description..

I do.

OK, what about?

rast3d -> 3draster
region3d -> to be removed (?)

       type   Data type(s)
              options: raster,3draster,vector,oldvector,asciivector,labels,
                       region,region3d,group,all
               raster: raster map(s)
               3draster: 3D raster map(s)
               vector: vector map(s)
               oldvector: old vector map(s) (GRASS 5.0)
               asciivector: ASCII vector map(s)
               labels: paint label file(s)
               region: region definition(s)
               group: imagery group(s)
               all: all types

[neteler]

Replying to martinl:

OK, what about?

rast3d -> 3draster
region3d -> to be removed (?)

Sounds like a good compromise! +1

[annakrat]

If you use 3draster in python as module option it won't work.

[martinl]

Replying to annakrat:

If you use 3draster in python as module option it won't work.

>>> g.read_command('g.list', type='3draster').splitlines()
['x']

?

[mlennert]

Just a heads up for this discussion and the actual implementation:

r63102 broke v.db.update because the key was changed in the parameter definitions, but the use of the key wasn't:

qcolumn key changed to query_column, but this was not done for the parsing of the parameter content:

qcolumn = optionsqcolumn

I corrected it in r63211 and r63212, but please watch out for this.

Moritz

[glynn]

Replying to annakrat:

We shouldn't forget that although unabbreviating doesn't cause problems in the command line, python scripts require the full name of the command.

PyGRASS may require that; grass.script.run_command() etc don't.

[annakrat]

Replying to martinl:

Replying to annakrat:

If you use 3draster in python as module option it won't work.

>>> g.read_command('g.list', type='3draster').splitlines()
['x']

?

I wrote 'as module option'. If you will make change to the type, you should make the change to the option name too and this will not work:

>>> gscript.run_command('g.region', 3draster='xxx')
 File "<stdin>", line 1
   gscript.run_command('g.region', 3draster='xxx')
                                          ^
SyntaxError: invalid syntax

But otherwise, I like this better than 'volume', so perhaps we could find some workaround for Python scripts and use underscore:

>>> gscript.run_command('g.region', _3draster='xxx')

This should work, although the underscore before was marked as obsolete, in favor of underscore after (for python keywords like map, input). But we can make an exception for keywords starting with numbers.

[martinl]

Replying to mlennert:

I corrected it in r63211 and r63212, but please watch out for this.

thanks anyway I am going to fix scripts and manuals using a script based on source:grass/trunk/lib/gis/renamed_options​.

[martinl]

Replying to annakrat:

But otherwise, I like this better than 'volume', so perhaps we could find some workaround for Python scripts and use underscore:

>>> gscript.run_command('g.region', _3draster='xxx')

This should work, although the underscore before was marked as obsolete, in favor of underscore after (for python keywords like map, input). But we can make an exception for keywords starting with numbers.

OK, I changed element name from 'volume' to '3draster' in r63218.

[martinl]

Replying to martinl:

Replying to martinl:

in r63189 I renamed

btw, do we need region3d ? ASAIK the region is 3D by default. Martin

removed in r63236. Now we have

               raster: raster map(s)
               3draster: 3D raster map(s)
               vector: vector map(s)
               oldvector: old  (GRASS 5.0) vector map(s)
               asciivector: ASCII vector map(s)
               labels: paint label file(s)
               region: region definition(s)
               group: imagery group(s)
               all: all types

[huhabla]

I am not against renaming the options, but IMHO its completely unnecessary. However, if the majority thinks it is necessary then i am fine with that.

But the recent changes in the option naming broke the temporal framework and as a result many temporal modules and tests.

Why??!!

If such invasive changes are performed, then i would expect that all the consequences of these changes are recognized and fixed before the commit!

We have a wonderful testsuite, so please use it to detect possible issues. Please do not simply break GRASS and expect that other developers will fix it.

[martinl]

Replying to huhabla:

If such invasive changes are performed, then i would expect that all the consequences of these changes are recognized and fixed before the commit!

I will fix it today.

[wenzeslaus]

Replying to martinl:

Replying to huhabla:

If such invasive changes are performed, then i would expect that all the consequences of these changes are recognized and fixed before the commit!

I will fix it today.

Before doing so, I would suggest to re-think the change, especially the one for 3D raster. We don't have an agreement and I even don't see what you are currently trying to implement:

> d.legend --h 2>&1 | grep 3D
raster3d   Name of 3D raster map
> g.region --h 2>&1 | grep 3D
3draster   Set region to match 3D raster map(s) (both 2D and 3D values)
res3   3D grid resolution (north-south, east-west and top-bottom)

Moreover, I'm not sure what do you want to do with module names such as d.rast, r.to.rast3 and all temporal datatypes.

Please, react also to the notes made by me, Anna and Soeren about the need for the change and the motivation behind the decisions for change of rast3d to raster3d/3draster/volume. From what was said I would especially point out that it seems to me that possibility to shorten the options is valued much more than documentation consistency and Python interface.

[wenzeslaus]

Martin and Markus, maybe I should add, just to clarify, that I'm definitively in favor of all other changes you are doing such as dsn -> output, output_prefix -> output, devi -> deviations and minimumtemperature -> minimum_temperature. I'm just very uncomfortable with changes related to rast/vect/rast3d, what you then imply for 3D raster and that you don't consider also module names.

[neteler]

Replying to martinl:

Replying to mlennert:

I corrected it in r63211 and r63212, but please watch out for this.

thanks anyway I am going to fix scripts and manuals using a script based on source:grass/trunk/lib/gis/renamed_options​.

For the record: Martin fixed a lot in r63261 (+ backported). Thanks.

Personally I would be happy to see more people submitting examples (...I wrote many).

[martinl]

Replying to wenzeslaus:

rast3d to raster3d/3draster/volume. From what was said I would especially point out that it

another option could to my mind. To use 'raster3d' and modify the parser in the way that it would support also 'rast' and 'rast3d' with appropriate warning

g.list rast,rast3d
WARNING: Option <rast> has been renamed to <raster>. Please update your script.
WARNING: Option <rast3d> has been renamed to <raster3d>. Please update your script.
raster maps...
3d raster maps...

What do you think about this option?

[martinl]

Replying to martinl:

another option could to my mind. To use 'raster3d' and modify the parser in the way that it would support also 'rast' and 'rast3d' with appropriate warning

> g.list rast,rast3d
> WARNING: Option <rast> has been renamed to <raster>. Please update your script.
> WARNING: Option <rast3d> has been renamed to <raster3d>. Please update your script.
> raster maps...
> 3d raster maps...

Personally I am still for '3draster' (and to solve PyGRASS issue raised by Anna), so

 g.list rast,rast3d
 WARNING: Option <rast> has been renamed to <raster>. Please update your script.
 WARNING: Option <rast3d> has been renamed to <3draster>. Please update your script.
 raster maps...
 3d raster maps...

[glynn]

Replying to martinl:

another option could to my mind. To use 'raster3d' and modify the parser in the way that it would support also 'rast' and 'rast3d' with appropriate warning

I don't think that "rast" should generate a warning. We want the user to be able to abbreviate option names when running commands interactively. At the same time, it's preferable for scripts to avoid abbreviations, which they can't do if the actual option name is itself an abbreviation.

Perhaps we should relax the rule relating to ambiguous abbreviations for the case where one option name is a prefix of another?

At present, if a module has options named "raster" and "raster3d", "rast" (or any other abbreviation of "raster") will be rejected as ambiguous. The option-matching function has to explicitly check for the case where an option name is given in full, otherwise even the unabbreviated option name would be rejected as ambiguous.

The change wouldn't be trivial; the function can no longer simply count matches, it has to record them. But as a side-effect of that, the error message for ambiguous options could list all of the matches.

If we don't make that change, then we should try to avoid the situation where one option name is a prefix of another. In particular, that precludes "raster3d" or "rast3d".

[cmbarton]

Could it cause a problem somewhere down the line to have this term beginning with a number--e.g. If it is used to name a temp file or something?

[annakrat]

Replying to cmbarton:

Could it cause a problem somewhere down the line to have this term beginning with a number--e.g. If it is used to name a temp file or something?

As I already said, it causes problems for Python because keyword parameter can't start with number. This is solvable by adding underscore and some special handling of this case, which is partially there already. It violates pep8.

Otherwise, I am not sure if we can expect some problems with temporal database with 3D raster time series. If the column names stay as they are now, it shouldn't be problem I think?

[cmbarton]

Given this, raster3d seems preferable. I suppose we could switch all to "volume", which was used inconsistently at some time in the past. Or perhaps "voxel"?

Michael

[glynn]

Replying to annakrat:

Replying to cmbarton:

Could it cause a problem somewhere down the line to have this term beginning with a number--e.g. If it is used to name a temp file or something?

As I already said, it causes problems for Python because keyword parameter can't start with number. This is solvable by adding underscore and some special handling of this case, which is partially there already. It violates pep8.

PEP8 is a style guide. There is no inherent reason why an argument name cannot start with an underscore. And we're not even talking about explicit arguments; such arguments will only ever be obtained via the kwargs mechanism.

In fact, I think that this is why I chose to use a leading underscore rather than a trailing underscore.

Still, I'd rather avoid having option names start with a digit. But unless we relax the ambiguity check, it wouldn't outweigh my preference to avoid using an option name which has a very common option name (rast or raster) as a prefix.

[wenzeslaus]

Replying to glynn:

Replying to annakrat:

Replying to cmbarton:

Could it cause a problem somewhere down the line to have this term beginning with a number--e.g. If it is used to name a temp file or something?

As I already said, it causes problems for Python because keyword parameter can't start with number. This is solvable by adding underscore and some special handling of this case, which is partially there already. It violates pep8.

PEP8 is a style guide. There is no inherent reason why an argument name cannot start with an underscore. And we're not even talking about explicit arguments; such arguments will only ever be obtained via the kwargs mechanism.

The problem may come once you want to use parameter as an variable or member variable. In later case underscore would means private which is technique not limited to Python. I'm also afraid that this can hit us or somebody else in some other language or system. Almost nothing allows numbers at the beginning of identifiers. I also think that for 3D raster it is much more probable that you hit this issue. For example, how should I name variable in my script which holds 3D raster map name or its maximum? _3draster_name? _3draster_max? I can of course name my variables whatever I want but wouldn't we stick to rast3d or raster3d in the GRASS source code anyway?

In fact, I think that this is why I chose to use a leading underscore rather than a trailing underscore.

Still, I'd rather avoid having option names start with a digit. But unless we relax the ambiguity check, it wouldn't outweigh my preference to avoid using an option name which has a very common option name (rast or raster) as a prefix.

I'm glad you are saying that. I think it is really important to state the priorities and motivations. If we just want backwards compatibility, then some special check in the parser can handle old short option names. And if we value the most backwards compatibility and short options, we probably should not not shorten at all in these special cases (type names).

Perhaps it is useful to ask why we want short options. It is for manual typing? Well then we perhaps should use techniques used elsewhere. And we are actually partially doing it. There is IDE-like auto-complete in GUI, Python dir completed is implemented for PyGRASS module interface and of course Linux command line auto-completes module names. So why not to take it further and auto-complete also parameters and perhaps other things by implementing auto-complete for shell?

Classic unix-like command line is anyway the only place where short options really matter if you consider the things above and also that you should not use shortened option names in scripts because it is not readable (that's why we are unabbreviating them, right?).

Perhaps we don't have to unabbreviate everything. It seems to me that there is no will to unabbreviate options for g.region or module names containing rast, vect or rast3d. I'm for explicit long descriptive option names but if it creates more issues then it solves (3draster) and if everybody would be using the shortened version all the time anyway (rast, ...), I prefer not to change them.

If we want short options for whatever reason, let's standardize them, rather than standardize the long options and provide ways how to avoid the standard.

[hcho]

Replying to wenzeslaus:

Replying to glynn:

Replying to annakrat:

Replying to cmbarton:

Could it cause a problem somewhere down the line to have this term beginning with a number--e.g. If it is used to name a temp file or something?

As I already said, it causes problems for Python because keyword parameter can't start with number. This is solvable by adding underscore and some special handling of this case, which is partially there already. It violates pep8.

PEP8 is a style guide. There is no inherent reason why an argument name cannot start with an underscore. And we're not even talking about explicit arguments; such arguments will only ever be obtained via the kwargs mechanism.

The problem may come once you want to use parameter as an variable or member variable. In later case underscore would means private which is technique not limited to Python. I'm also afraid that this can hit us or somebody else in some other language or system. Almost nothing allows numbers at the beginning of identifiers. I also think that for 3D raster it is much more probable that you hit this issue. For example, how should I name variable in my script which holds 3D raster map name or its maximum? _3draster_name? _3draster_max? I can of course name my variables whatever I want but wouldn't we stick to rast3d or raster3d in the GRASS source code anyway?

In fact, I think that this is why I chose to use a leading underscore rather than a trailing underscore.

Still, I'd rather avoid having option names start with a digit. But unless we relax the ambiguity check, it wouldn't outweigh my preference to avoid using an option name which has a very common option name (rast or raster) as a prefix.

I'm glad you are saying that. I think it is really important to state the priorities and motivations. If we just want backwards compatibility, then some special check in the parser can handle old short option names. And if we value the most backwards compatibility and short options, we probably should not not shorten at all in these special cases (type names).

Perhaps it is useful to ask why we want short options. It is for manual typing? Well then we perhaps should use techniques used elsewhere. And we are actually partially doing it. There is IDE-like auto-complete in GUI, Python dir completed is implemented for PyGRASS module interface and of course Linux command line auto-completes module names. So why not to take it further and auto-complete also parameters and perhaps other things by implementing auto-complete for shell?

Classic unix-like command line is anyway the only place where short options really matter if you consider the things above and also that you should not use shortened option names in scripts because it is not readable (that's why we are unabbreviating them, right?).

Perhaps we don't have to unabbreviate everything. It seems to me that there is no will to unabbreviate options for g.region or module names containing rast, vect or rast3d. I'm for explicit long descriptive option names but if it creates more issues then it solves (3draster) and if everybody would be using the shortened version all the time anyway (rast, ...), I prefer not to change them.

I strongly agree with you. Personally, I'm fine with the old type names. If shortening rast3d= is an issue because of conflicts with rast=, we could change it to volume= or voxel= (and vo.* module names). I don't think it's a good idea to have to type an underscore before certain option names in Python or possibly other languages and we could avoid such annoying/inconsistent situations by naming elements more carefully.

If we want short options for whatever reason, let's standardize them, rather than standardize the long options and provide ways how to avoid the standard.

+1

[glynn]

Replying to wenzeslaus:

Perhaps we don't have to unabbreviate everything. It seems to me that there is no will to unabbreviate options for g.region or module names containing rast, vect or rast3d.

FWIW, I would like to unabbreviate everything which could reasonably be unabbreviated (including raster and vector rather than rast and vect). Clearly, there are cases where this would be unreasonable (e.g. option names may contain acronyms which could expand to several words, some of which may be long compound words, or may themselves be acronyms).

I'm for explicit long descriptive option names but if it creates more issues then it solves (3draster) and if everybody would be using the shortened version all the time anyway (rast, ...), I prefer not to change them.

Code should always use the full name. Abbreviations are intended for interactive use.

[glynn]

Replying to hcho:

I don't think it's a good idea to have to type an underscore before certain option names in Python or possibly other languages and we could avoid such annoying/inconsistent situations by naming elements more carefully.

There are two reasons why an option name cannot be used in Python. One is if it doesn't conform to the syntax for an identifier (e.g. starts with a digit). The other is if it is a reserved word (e.g. "from").

Avoiding the first case is simple enough. A requirement that option names begin with a letter and consist solely of letters, digits and underscores will ensure conformance with the identifier syntax for any common language.

The second one is more awkward, as different languages have different sets of reserved words. The only real constant is that many of them are quite common words which would frequently be an ideal choice for an option name.

I'm not sure that we should reject such names simply because they can't be used as a variable/parameter/etc name in a specific language without a tweak such as adding an underscore (or changing case; now that option names are limited to lower-case, having grass.script.make_command() force each name to lower case would be feasible).

[wenzeslaus]

Replying to glynn:

Replying to wenzeslaus:

I'm for explicit long descriptive option names but if it creates more issues then it solves (3draster) and if everybody would be using the shortened version all the time anyway (rast, ...), I prefer not to change them.

Code should always use the full name. Abbreviations are intended for interactive use.

This is what I mean. If we want better interactive use, let's improve it but not at expense of writing the code. (3draster is improvement for interactive use but it makes code worse by requiring _3draster.)

As I said before, short options are mostly advantageous for unix-like command lines, so let's use what it used there, the Tab key auto-complete. This is definitively not an obsolete thing, for example Git is using it extensively, SVN supports it too.

Another solution for abbreviating raster and raster3d which solves the problem at the level of abbreviating and does not make the code worse (although it might harm a little bit) is changing the ambiguity rules as Glynn suggested in comment 75. My first impression from shortening actually was that it counts matching letters for each option and gives a best match. But this is not (unlike Tab key auto-complete) completely isolated from calling the modules from some code, so I'm afraid of two things. First, that it can become dangerous for people not using the long options in the code (e.g. for compatibility reasons). Second, that the option matching can become so costly that it would actually make module calls more expensive.

[huhabla]

I full agree with Vaclav.

However, the modification of GRASS was already performed, regardless the fact that we still discuss the renaming of rast, rast3d and vect options.

This, as i already stated before, completely broke the temporal framework. It is not only a matter of renaming option names, the whole internal temporal data type system and handling of different map types was broken because of this. What to do now with temporal module names? Must t.rast.* modules be renamed into t.raster.*? Or t.rast3d.* modules into t.3draster.*?

Someone tried to fix the framework by simply renaming rast3d into 3draster, but this did not solved the problem. The framework was still broken. Why was the testsuite not used to check these changes? Why do we have the test-framework anyway? Just a tiny hint: renaming the option "n" from v.random into "npoints" broke plenty of temporal vector module tests, since they are using PyGRASS to call v.random and PyGRASS can't handle abbreviations.

I needed 5 hours to fix the temporal framework and i am pretty sure that i did not catch all issues that were introduced with the renaming.

Putting GRASS in such broken a state without using the testuite to write and use tests or at least to verify the changes with existing tests is IMHO irresponsible. Especially if we plan to make a beta release.

I am sorry for my harsh words, but i had to invest 5h for no good reason to keep on with my regular work.

Replying to wenzeslaus:

Replying to glynn:

Replying to annakrat:

Replying to cmbarton:

Could it cause a problem somewhere down the line to have this term beginning with a number--e.g. If it is used to name a temp file or something?

As I already said, it causes problems for Python because keyword parameter can't start with number. This is solvable by adding underscore and some special handling of this case, which is partially there already. It violates pep8.

PEP8 is a style guide. There is no inherent reason why an argument name cannot start with an underscore. And we're not even talking about explicit arguments; such arguments will only ever be obtained via the kwargs mechanism.

The problem may come once you want to use parameter as an variable or member variable. In later case underscore would means private which is technique not limited to Python. I'm also afraid that this can hit us or somebody else in some other language or system. Almost nothing allows numbers at the beginning of identifiers. I also think that for 3D raster it is much more probable that you hit this issue. For example, how should I name variable in my script which holds 3D raster map name or its maximum? _3draster_name? _3draster_max? I can of course name my variables whatever I want but wouldn't we stick to rast3d or raster3d in the GRASS source code anyway?

In fact, I think that this is why I chose to use a leading underscore rather than a trailing underscore.

Still, I'd rather avoid having option names start with a digit. But unless we relax the ambiguity check, it wouldn't outweigh my preference to avoid using an option name which has a very common option name (rast or raster) as a prefix.

I'm glad you are saying that. I think it is really important to state the priorities and motivations. If we just want backwards compatibility, then some special check in the parser can handle old short option names. And if we value the most backwards compatibility and short options, we probably should not not shorten at all in these special cases (type names).

Perhaps it is useful to ask why we want short options. It is for manual typing? Well then we perhaps should use techniques used elsewhere. And we are actually partially doing it. There is IDE-like auto-complete in GUI, Python dir completed is implemented for PyGRASS module interface and of course Linux command line auto-completes module names. So why not to take it further and auto-complete also parameters and perhaps other things by implementing auto-complete for shell?

Classic unix-like command line is anyway the only place where short options really matter if you consider the things above and also that you should not use shortened option names in scripts because it is not readable (that's why we are unabbreviating them, right?).

Perhaps we don't have to unabbreviate everything. It seems to me that there is no will to unabbreviate options for g.region or module names containing rast, vect or rast3d. I'm for explicit long descriptive option names but if it creates more issues then it solves (3draster) and if everybody would be using the shortened version all the time anyway (rast, ...), I prefer not to change them.

If we want short options for whatever reason, let's standardize them, rather than standardize the long options and provide ways how to avoid the standard.

[martinl]

Replying to huhabla:

I needed 5 hours to fix the temporal framework and i am pretty sure that i did not catch all issues that were introduced with the renaming.

sorry for that, *BUT* we were speaking about consolidation of parameters *MANY YEARS* ago! But *NOTHING* really *CHANGED* till now. I agree with you that we are too late with that but I still think that it made sense to do it.

Putting GRASS in such broken a state without using the testuite to write and use tests or at least to verify the changes with existing tests is IMHO irresponsible. Especially if we plan to make a beta release.

See my note above...

[huhabla]

Replying to martinl:

Replying to huhabla:

I needed 5 hours to fix the temporal framework and i am pretty sure that i did not catch all issues that were introduced with the renaming.

sorry for that, *BUT* we were speaking about consolidation of parameters *MANY YEARS* ago! But *NOTHING* really *CHANGED* till now. I agree with you that we are too late with that but I still think that it made sense to do it.

Putting GRASS in such broken a state without using the testuite to write and use tests or at least to verify the changes with existing tests is IMHO irresponsible. Especially if we plan to make a beta release.

See my note above...

Dear Martin, many thanks for all of your effort.

I am not arguing about the sense of the changes. As i stated before i am fine with them. I simply have a problem how these changes are applied. We have a wonderful test suite that will help you to avoid breaking parts of GRASS. We should make use of it to avoid such a discussion.

[martinl]

Replying to huhabla:

many thanks for all of your effort.

personally I am not happy with these changes too. They should have been done much more earlier, now it's very late. But still I think it's better than not to do it at all. Anyway I am not happy about this situation.

I am not arguing about the sense of the changes. As i stated before i am fine with them. I simply have a problem how these changes are applied. We have a wonderful test suite that will help you to avoid breaking parts of GRASS. We should make use of it to avoid such a discussion.

Let's try to focus on element list name. Under condition that we don't want to use abbreviated names, what is your preference

raster3d*
volume
3draster

(*) Then we could modify parser to understand also rast and rast3d with appropriate warning

g.list rast
WARNING: Element <rast> has been renamed <raster>. Please update your script.
...

g.list rast3d
WARNING: Element <rast3d> has been renamed <raster3d>. Please update your script.
...

What do you think?

[glynn]

Replying to wenzeslaus:

As I said before, short options are mostly advantageous for unix-like command lines, so let's use what it used there, the Tab key auto-complete.

We shouldn't rely upon this feature being available. And even when it is, it's not automatically better than abbreviation (e.g. completion is affected by latency, abbreviation isn't).

[glynn]

Replying to martinl:

Let's try to focus on element list name. Under condition that we don't want to use abbreviated names, what is your preference

raster3d*
volume
3draster

My preference would be "volume"; the other two have issues. We can debate forever about how significant those issues are, but "volume" avoids them entirely.

[wenzeslaus]

Replying to glynn:

Replying to martinl:

Let's try to focus on element list name. Under condition that we don't want to use abbreviated names, what is your preference

raster3d*
volume
3draster

My preference would be "volume"; the other two have issues. We can debate forever about how significant those issues are, but "volume" avoids them entirely.

volume avoids issues with naming variables but creates issues when talking with people. From my experience, volume is very confusing while 3D raster (-> raster3d) works because it is very explanatory and it is not a ​homonym. Voxel seems not be an option anymore but it is a term which people react to time to time because it is at least clear that it is something special (not a ​volume in liters).

BTW, Wikipedia is not really helpful in deciding about volume/voxel/raster3d but it links not really related ​Raster3D software (​Raster graphics) and it talks about 2D and 3D raster and vector when it is describing a book ​Computer Graphics: Principles and Practice which is highly related.

[annakrat]

Replying to glynn:

Replying to martinl:

Let's try to focus on element list name. Under condition that we don't want to use abbreviated names, what is your preference

raster3d*
volume
3draster

My preference would be "volume"; the other two have issues. We can debate forever about how significant those issues are, but "volume" avoids them entirely.

So if we decide for volume, do we plan to rename all modules like this? If not, we should stay with raster3d. Otherwise it will get very confusing in my opinion.

r3.info -> vo.info
r.to.rast3-> t.to.volume

I must say I am not particularly fond of vo.* modules, but maybe it's just a matter of habit.

[hcho]

Replying to glynn:

Replying to martinl:

Let's try to focus on element list name. Under condition that we don't want to use abbreviated names, what is your preference

raster3d*
volume
3draster

My preference would be "volume"; the other two have issues. We can debate forever about how significant those issues are, but "volume" avoids them entirely.

+1 for volume (+2 for voxel) for the exact same reason.

[martinl]

Replying to hcho:

+1 for volume (+2 for voxel) for the exact same reason.

-1 for voxel, than we could rename 'raster' to 'pixel' ;-)

I am still not sure we need to rename than all modules from r3 to vo...

[helena]

Replying to martinl:

Replying to hcho:

+1 for volume (+2 for voxel) for the exact same reason.

-1 for voxel, than we could rename 'raster' to 'pixel' ;-)

I am still not sure we need to rename than all modules from r3 to vo...

so is there also an agreement to rename all r3.* commands to vo.* commands and changing all manuals accordingly? If yes, this will require several weeks of focused testing and reviews/updates of all grass7 related manuals, tutorials and scripts.I don't have a good solution for this issue but I am not a big fan of changing names unless it is absolutely necessary or leads to a major improvement. Reading through the discussion I am wondering whether anybody has done a solid research on what are the 3D rasters referred to in different disciplines and which term is closest to our users backgrounds or are all these three names that are being considered based on what each individual thinks is right? Which brings me to another point - is this being discussed on the users list as well? The users will be the ones most affected (and potentially confused) so why not get some feedback from them?

[martinl]

Replying to helena:

-1 for voxel, than we could rename 'raster' to 'pixel' ;-)

I am still not sure we need to rename than all modules from r3 to vo...

so is there also an agreement to rename all r3.* commands to vo.* commands and changing all manuals accordingly? If yes, this will require several weeks of focused testing and

not at all, personally I don't like to stick with shortened element names (rast, rast3d) just because it makes our life easier. Taking in account a real world it seems to me that the reasonable is to rename rast -> raster and rast3d -> raster3d and to modify the parser to understand also shortened element names.

[cmbarton]

Can raster_3D be better parsed in long and shortened form?

Michael

[annakrat]

Replying to cmbarton:

Can raster_3D be better parsed in long and shortened form?

It has the same issues as raster3d, still rast is ambiguous. You can shorten raster_3D to rast3d or r3 but you can't shorten raster.

[huhabla]

Replying to martinl:

Replying to helena:

-1 for voxel, than we could rename 'raster' to 'pixel' ;-)

I am still not sure we need to rename than all modules from r3 to vo...

so is there also an agreement to rename all r3.* commands to vo.* commands and changing all manuals accordingly? If yes, this will require several weeks of focused testing and

not at all, personally I don't like to stick with shortened element names (rast, rast3d) just because it makes our life easier. Taking in account a real world it seems to me that the reasonable is to rename rast -> raster and rast3d -> raster3d and to modify the parser to understand also shortened element names.

This would be absolutely my favorite solution.

IMHO renaming all 3D raster and STR3DS modules using "volume" name approach will result in a lot of unnecessary work and inconsistency. As a result all documentation in code and manual pages, the module help, examples and tests must be updated/modified as well. Not speaking about the confusion with wording in already published scientific papers and books ... .

I don't think this is worth the effort.

[martinl]

Replying to huhabla:

not at all, personally I don't like to stick with shortened element names (rast, rast3d) just because it makes our life easier. Taking in account a real world it seems to me that the reasonable is to rename rast -> raster and rast3d -> raster3d and to modify the parser to understand also shortened element names.

This would be absolutely my favorite solution.

please could you try attachment:raster3d.diff​. Does it make sense to you?

[glynn]

Replying to martinl:

Does it make sense to you?

Not really. I'd much prefer a general solution, i.e. if an abbreviation matches multiple options, and one of them is a prefix of all of the others, the abbreviation should be deemed to unambiguously refer to the prefix option.

So e.g. if a module has "raster" and "raster_3d" options, "r", "rast" and "raster" would all be deemed to unambiguously refer to "raster", while any abbreviation intended to refer to "raster_3d" would have to include the "3" (e.g. "r3", "r3d", "rast3", "rast3d", ...) to ensure that it cannot also match "raster".

[wenzeslaus]

Replying to huhabla:

Replying to martinl:

Replying to helena:

-1 for voxel, than we could rename 'raster' to 'pixel' ;-)

I am still not sure we need to rename than all modules from r3 to vo...

so is there also an agreement to rename all r3.* commands to vo.* commands and changing all manuals accordingly? If yes, this will require several weeks of focused testing and

not at all, personally I don't like to stick with shortened element names (rast, rast3d) just because it makes our life easier. Taking in account a real world it seems to me that the reasonable is to rename rast -> raster and rast3d -> raster3d and to modify the parser to understand also shortened element names.

This would be absolutely my favorite solution.

I like raster and raster3d.

For example, there will be no need to change grass.script.raster3d (which would end up as grass.script._3draster in the other case). From this point of view raster_3d is also not good because underscore should not be in the name of a Python module if possible and moreover I don't like raster_3d in general. Shortening possibilities are not as important to me as name without underscore.

What about the module names? Are we changing that? Or is raster3d just rast3d in module names?

[martinl]

Replying to wenzeslaus:

I like raster and raster3d.

From all noted options I like this one too.

What about the module names? Are we changing that? Or is raster3d just rast3d in module names?

No strong opinion here, I would keep current module names.

[neteler]

Should we vote on this topic? We really need to converge asap since beta4 is overdue.

[martinl]

Replying to neteler:

Should we vote on this topic? We really need to converge asap since beta4 is overdue.

I fully agree, we need to somehow decide this issue...

[martinl]

Replying to wenzeslaus:

For example, there will be no need to change grass.script.raster3d (which would end up as grass.script._3draster in the other case). From this point of view raster_3d is also not good because underscore should not be in the name of a Python module if possible and moreover I don't like raster_3d in general. Shortening possibilities are not as important to me as name without underscore.

I would agree with that.

@glynn: please could you review your opinion on already attached patch. I understand that you prefer some generic solution. On the other hand we need to decide somehow (to find a compromise) since we would like to see GRASS7 released in reasonable time.

[martinl]

Replying to glynn:

So e.g. if a module has "raster" and "raster_3d" options, "r", "rast" and "raster" would all be deemed to unambiguously refer to "raster", while any abbreviation intended to refer to "raster_3d" would have to include the "3" (e.g. "r3", "r3d", "rast3", "rast3d", ...) to ensure that it cannot also match "raster".

I have prepared for testing a new patch based on your suggestion attachment:raster_3d.diff​

Then

g.list rast3d

will work. The patch contains modification of the parser to understand rast

g.list rast
WARNING: Please update the usage of <g.list>: option <rast> has been renamed to <raster>
...

[martinl]

Replying to wenzeslaus:

For example, there will be no need to change grass.script.raster3d (which would end up as grass.script._3draster in the other case). From this point of view raster_3d is also not good because underscore should not be in the name of a Python module if possible and moreover I don't like raster_3d in general. Shortening possibilities are not as important to me as name without underscore.

What about the module names? Are we changing that? Or is raster3d just rast3d in module names?

Personally I would tend to rename just

• element name raster_3d (or raster3d)
• module options e.g. d.legend raster_3d

I would not touch names of the modules, some of them already use shorten names, eg. d.rast.leg, so I would keep rast3d. Similarly I would not touch python module names.

Any opinion from other devs?

[glynn]

Replying to martinl:

@glynn: please could you review your opinion on already attached patch. I understand that you prefer some generic solution. On the other hand we need to decide somehow (to find a compromise) since we would like to see GRASS7 released in reasonable time.

It's preferable to doing nothing (i.e. leaving modules using abbreviated forms), and is compatible with the result obtained by relaxing the ambiguity rules.

Should we do something similar for option names? E.g. g.region, g.rename, g.copy have rast= and rast3d= options.

[martinl]

Replying to glynn:

Should we do something similar for option names? E.g. g.region, g.rename, g.copy have rast= and rast3d= options.

yes, enough would to register them in source:grass/trunk/lib/gis/renamed_options​

[annakrat]

Replying to martinl:

Personally I would tend to rename just

• element name raster_3d (or raster3d)
• module options e.g. d.legend raster_3d

I would not touch names of the modules, some of them already use shorten names, eg. d.rast.leg, so I would keep rast3d. Similarly I would not touch python module names.

Any opinion from other devs?

I don't really like the underscore, I would go with raster3d, I think we can manage to type raster3d because 3D rasters are generally not used as often as rasters.

I would keep the names of modules as they are now.

[mlennert]

Replying to martinl:

Replying to wenzeslaus:

For example, there will be no need to change grass.script.raster3d (which would end up as grass.script._3draster in the other case). From this point of view raster_3d is also not good because underscore should not be in the name of a Python module if possible and moreover I don't like raster_3d in general. Shortening possibilities are not as important to me as name without underscore.

What about the module names? Are we changing that? Or is raster3d just rast3d in module names?

Personally I would tend to rename just

• element name raster_3d (or raster3d)
• module options e.g. d.legend raster_3d

I would not touch names of the modules, some of them already use shorten names, eg. d.rast.leg, so I would keep rast3d. Similarly I would not touch python module names.

Any opinion from other devs?

+1

Moritz

[hcho]

Replying to annakrat:

Replying to martinl:

Personally I would tend to rename just

• element name raster_3d (or raster3d)
• module options e.g. d.legend raster_3d

I would not touch names of the modules, some of them already use shorten names, eg. d.rast.leg, so I would keep rast3d. Similarly I would not touch python module names.

Any opinion from other devs?

I don't really like the underscore, I would go with raster3d, I think we can manage to type raster3d because 3D rasters are generally not used as often as rasters.

I agree. I don't like the underscore either. Maybe, the parser could treat numbers just like the underscore so that we can shorten raster3d as r3.

I would keep the names of modules as they are now.

Like Martin mentioned, I don't think we need to change module names if we go for raster3d because other modules already use short names.

[glynn]

Replying to hcho:

I don't really like the underscore, I would go with raster3d, I think we can manage to type raster3d because 3D rasters are generally not used as often as rasters.

I agree. I don't like the underscore either. Maybe, the parser could treat numbers just like the underscore so that we can shorten raster3d as r3.

Having the parser treat numbers as separate words shouldn't be a problem.

OTOH, while I can understand why people might not want to use e.g. "back_ground" (to allow abbreviation as "bg"), I really don't see the problem with "raster_3d". To me, "raster" and "3d" are entirely separate words.

[neteler]

Dear devs:

I see the problem of no consensus here. Do we need to fork GRASS at this point or postpone the release forever? (irony flag on or off as you wish)

[wenzeslaus]

Replying to neteler:

I see the problem of no consensus here. Do we need to fork GRASS at this point or postpone the release forever? (irony flag on or off as you wish)

My understanding was that we have or we are very close to consensus. I the last round, martinl suggested "raster and (raster_3d or raster3d)" for options and elements (when leaving module names as they are). mlennert agrees with not renaming module names (if I understand correctly).

annakrat and hcho don't want the underscore for 3D raster, so this changes "raster and (raster_3d and raster3d)" just to "raster and raster3d". I already said that I prefer raster3d over raster_3d. In my opinion such a common option name as one for 3D raster (common for me and name of one of the basic types) should not have an underscore. But glynn does not see the point in not using underscore in case of raster_3d. So, this is the only unclear part depending on how strong is Glynn's opinion.

hcho suggests to allow shortening of options also where the numbers are. glynn says that's possible.

I hope I'm not misquoting anybody.

[mlennert]

Replying to wenzeslaus:

Replying to neteler:

I see the problem of no consensus here. Do we need to fork GRASS at this point or postpone the release forever? (irony flag on or off as you wish)

My understanding was that we have or we are very close to consensus. I the last round, martinl suggested "raster and (raster_3d or raster3d)" for options and elements (when leaving module names as they are). mlennert agrees with not renaming module names (if I understand correctly).

annakrat and hcho don't want the underscore for 3D raster, so this changes "raster and (raster_3d and raster3d)" just to "raster and raster3d". I already said that I prefer raster3d over raster_3d. In my opinion such a common option name as one for 3D raster (common for me and name of one of the basic types) should not have an underscore. But glynn does not see the point in not using underscore in case of raster_3d. So, this is the only unclear part depending on how strong is Glynn's opinion.

Why only Glynn's ? For me that question goes both ways. I have to admit that I don't understand the opposition to raster_3d. Is this only based on aesthetics ? In terms of ease of programming and usability, I have the feeling that raster_3d is superior as it allows you to use the current parser behaviour to use abbreviations like r3 which raster3d doesn't.

More generally, this whole issue shows to me what happens in an unorganised release process where we start to release betas without being sure that we are in a stable enough state to do so and then begin such a fundamental review such as this one very late into the process...

Moritz

[wenzeslaus]

Replying to mlennert:

Replying to wenzeslaus:

Replying to neteler:

I see the problem of no consensus here. Do we need to fork GRASS at this point or postpone the release forever? (irony flag on or off as you wish)

My understanding was that we have or we are very close to consensus. I the last round, martinl suggested "raster and (raster_3d or raster3d)" for options and elements (when leaving module names as they are). mlennert agrees with not renaming module names (if I understand correctly).

annakrat and hcho don't want the underscore for 3D raster, so this changes "raster and (raster_3d and raster3d)" just to "raster and raster3d". I already said that I prefer raster3d over raster_3d. In my opinion such a common option name as one for 3D raster (common for me and name of one of the basic types) should not have an underscore. But glynn does not see the point in not using underscore in case of raster_3d. So, this is the only unclear part depending on how strong is Glynn's opinion.

Why only Glynn's ? For me that question goes both ways. I have to admit that I don't understand the opposition to raster_3d.

So, sorry for misquoting you.

Is this only based on aesthetics ? In terms of ease of programming and usability, I have the feeling that raster_3d is superior as it allows you to use the current parser behaviour to use abbreviations like r3 which raster3d doesn't.

As I said before, I don't care much about abbreviations. I even consider them bad. They are bad for the source code and most of the scripting because of readability (I think we have a consensus on that). And they are bad for manuals and tutorials because they are harder for user to translate to GUI or QGIS (GUI or processing). So then I don't see the point in putting so much effort into the abbreviated options. And I also think that we cannot unabbreviate everything, some options would be just too long, too unpractical. Anyway, I can manage to write raster_3d everywhere and I will hope that everybody will do the same for all GRASS core code and addons. Perhaps it would be useful to have some environmental variable to disable abbreviating which could be used for testing (or to speed up the parsing).

[neteler]

Replying to mlennert:

More generally, this whole issue shows to me what happens in an unorganised release process

Not really: Martin, me and some others worked for *years* on the consolidation. Just others didn't care too much until now. Eg I went through the 4000 parameters and flags multiple times. Just join the party :-)

where we start to release betas without being sure that we are in a stable enough state to do so and then begin such a fundamental review such as this one very late into the process...

Concerning the beta releases: And I am very sure that without the beta releases the widespread testing which actually happens (also is possible thanks to the packagers preparing installers for the various OS'es and distros who need a tangible package) hadn't been done.

[cmbarton]

I've been dealing with end of semester and traveling. Wow! A lot of words spilled over a small suggestion to simplify parsing in code and in commands. FWIW, the underscore makes it a *little* easier to parse and potentially shorten (if that is desirable) the term for GRASS's unique 3D/voxel format. It also makes it a little easier to quickly differentiate the 2D raster things (raster and raster.x) from the 3D raster things (raster_3D and raster_3D.x). I don't find an underscore that much trouble to type, except on my iPad where I can't run GRASS anyway.

That said, it's not a big deal to me either way. Since one is very minimally more difficult to type than the other (1 character), I'd suggest whichever form makes programming easier. Maybe a simple vote is needed to move on. I'm delighted to go along with the majority so we release the next beta

[glynn]

Replying to wenzeslaus:

annakrat and hcho don't want the underscore for 3D raster, so this changes "raster and (raster_3d and raster3d)" just to "raster and raster3d". I already said that I prefer raster3d over raster_3d. In my opinion such a common option name as one for 3D raster (common for me and name of one of the basic types) should not have an underscore.

Can someone, anyone, explain what exactly is the problem with underscores? Why are they bad, why shouldn't we use them, etc?

To my mind, any name that is made of more than one word should have some kind of separator between the words. For GRASS option names, "some kind of separator" equates to an underscore.

[glynn]

Replying to mlennert:

More generally, this whole issue shows to me what happens in an unorganised release process where we start to release betas without being sure that we are in a stable enough state to do so and then begin such a fundamental review such as this one very late into the process...

It's always the same. If something can be left until later, it is. Once we reach the "last chance to comment" stage, all of the discussions which should have happened over the last half-dozen years start.

This isn't specific to GRASS. The standard that eventually became C++11 spent most of its life being referred to as "C++0x" on the assumption that it would be published by the end of 2009 at the latest.

It's something of a chicken-and-egg problem. You don't get adequate feedback until many people start using it, and many people won't start using it until it's perceived as stable.

[neteler]

Replying to glynn:

Can someone, anyone, explain what exactly is the problem with underscores? Why are they bad, why shouldn't we use them, etc?

To be it looks ok to change the element name "rast3d" to "raster_3d". If I am not wrong it solves all technical problems, too, including abbreviations etc.

[neteler]

Replying to neteler:

To be it looks ok to change the element name "rast3d" to "raster_3d". If I am not wrong it solves all technical problems, too, including abbreviations etc.

Opinions? Let's come to a decision, thanks!

[annakrat]

Replying to neteler:

Replying to neteler:

To be it looks ok to change the element name "rast3d" to "raster_3d". If I am not wrong it solves all technical problems, too, including abbreviations etc.

Opinions? Let's come to a decision, thanks!

I think most devs are OK with the underscore version. Personally, I don't like the underscore, but I don't have any powerful argument, so I will accept it.

[hcho]

Replying to annakrat:

Replying to neteler:

Replying to neteler:

To be it looks ok to change the element name "rast3d" to "raster_3d". If I am not wrong it solves all technical problems, too, including abbreviations etc.

Opinions? Let's come to a decision, thanks!

I think most devs are OK with the underscore version. Personally, I don't like the underscore, but I don't have any powerful argument, so I will accept it.

Agree. I don't like the underscore version, but I'm OK with raster_3d because it's at least better than 3draster or 3d_raster. Also, we don't have to rename any modules and the parser doesn't have to treat numbers as a special character.

[mlennert]

+1 for raster_3d.

Moritz

[lucadelu]

Replying to mlennert:

+1 for raster_3d.

+1 also for me

Moritz

[martinl]

Replying to mlennert:

+1 for raster_3d.

+1

I think that it would be useful to modify parser to understand also rast (`rast3d is OK because '_' is treated as word separator).

[martinl]

Replying to martinl:

I think that it would be useful to modify parser to understand also rast (`rast3d is OK because '_' is treated as word separator).

attachment:raster_3d.diff​

[hcho]

Replying to martinl:

Replying to martinl:

I think that it would be useful to modify parser to understand also rast (`rast3d is OK because '_' is treated as word separator).

attachment:raster_3d.diff​

I'm generally against adding special cases in the code. IMHO, if the user types rast, s/he means raster not raster_3d and it shouldn't be ambiguous. Currently, the parser would stop with a parameter ambiguous error. I think it would be more useful to modify the parser such that it takes the shortest unambiguous parameter with no further underscores.

For example,

• modules with parameters raster_map= and raster_3d=: rast and raster_ would be ambiguous.

• modules with parameters raster= and raster_3d=: r and rast would match with raster= and r3 and rast3 with raster_3d=

• modules with parameters raster_map= and raster_map_2=: rast ambiguous, rmap for raster_map= and rmap2 for raster_map_2=

• modules with parameters raster_map= and raster_map2=: rast ambiguous, rmap for raster_map= and rmap2 for raster_map2=

I'm not sure if I was clear enough here.

[cmbarton]

Replying to mlennert:

+1 for raster_3d.

Moritz

+1 for me

Michael

[glynn]

Replying to hcho:

• modules with parameters raster_map= and raster_map_2=: rast ambiguous, rmap for raster_map= and rmap2 for raster_map_2=

• modules with parameters raster_map= and raster_map2=: rast ambiguous, rmap for raster_map= and rmap2 for raster_map2=

I'm not sure if I was clear enough here.

You're suggesting that the abbreviation must include some part of the last word of the option name? Or that it must include the last character? I.e. is rm= ambiguous?

The main problems I foresee with that are

• The last word (or an abbreviation of it) might match some earlier portion of the option name, meaning that the user intended to specify (part of) the last word but the characters were matched against an earlier portion.

• It's possible for a string to match the last word of multiple options. E.g. given options raster_map and raster_2_map, rmap would match both equally.

[martinl]

Replying to cmbarton:

Replying to mlennert:

+1 for raster_3d.

Moritz

+1 for me

raster_3d introduced in r63584.

[martinl]

Replying to martinl:

raster_3d introduced in r63584.

If no objections I will backport modified element names including raster_3d to relbr70.

[hcho]

Replying to glynn:

Replying to hcho:

• modules with parameters raster_map= and raster_map_2=: rast ambiguous, rmap for raster_map= and rmap2 for raster_map_2=

• modules with parameters raster_map= and raster_map2=: rast ambiguous, rmap for raster_map= and rmap2 for raster_map2=

I'm not sure if I was clear enough here.

You're suggesting that the abbreviation must include some part of the last word of the option name? Or that it must include the last character? I.e. is rm= ambiguous?

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).

The main problems I foresee with that are

• The last word (or an abbreviation of it) might match some earlier portion of the option name, meaning that the user intended to specify (part of) the last word but the characters were matched against an earlier portion.

I'm not talking about skipping words and using the last word, so this is not the case.

• It's possible for a string to match the last word of multiple options. E.g. given options raster_map and raster_2_map, rmap would match both equally.

rmap= would match raster_map= "only" because you cannot skip 2.

Hope I'm clearer now.

[hcho]

Replying to hcho:

Replying to glynn:

Replying to hcho:

• modules with parameters raster_map= and raster_map_2=: rast ambiguous, rmap for raster_map= and rmap2 for raster_map_2=

• modules with parameters raster_map= and raster_map2=: rast ambiguous, rmap for raster_map= and rmap2 for raster_map2=

I'm not sure if I was clear enough here.

You're suggesting that the abbreviation must include some part of the last word of the option name? Or that it must include the last character? I.e. is rm= ambiguous?

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'.

If you have raster_map_1= and raster_map_2=, rm= would be ambiguous because raster_map is still ambiguous. If you have raster_map= and raster_map_2=, rm= expands to raster_map= and it's not ambiguous.

The main problems I foresee with that are

• The last word (or an abbreviation of it) might match some earlier portion of the option name, meaning that the user intended to specify (part of) the last word but the characters were matched against an earlier portion.

I'm not talking about skipping words and using the last word, so this is not the case.

• It's possible for a string to match the last word of multiple options. E.g. given options raster_map and raster_2_map, rmap would match both equally.

rmap= would match raster_map= "only" because you cannot skip 2.

Hope I'm clearer now.

[martinl]

Replying to martinl:

Replying to martinl:

raster_3d introduced in r63584.

If no objections I will backport modified element names including raster_3d to relbr70.

done in r63617.

[neteler]

More element name change related updates in r63619 and r63620 (shell scripts)

[hcho]

BTW, we don't rename oldvector and asciivector to old_vector and ascii_vector for the same reason? They are also two words.

[martinl]

Replying to 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.

[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"?

[martinl]

Replying to 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.

[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

[glynn]

Replying to 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 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).

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

[martinl]

Replying to martinl:

Replying to 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.

[neteler]

Replying to martinl:

Replying to martinl:

Replying to 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.

[martinl]

Replying to 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.

[neteler]

Replying to 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.

[neteler]

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

[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)/

[martinl]

Any objections to rename

bgcolor

to

bg_color

?

[martinl]

Replying to 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​ !!!

[martinl]

Replying to 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.

[neteler]

Replying to 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.

[hellik]

Replying to martinl:

Replying to 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.

[hellik]

Replying to martinl:

Replying to 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)

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

[martinl]

Replying to 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.

[neteler]

Replying to wenzeslaus:

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.

Sounds good to me.

[martinl]

For record: in r63704 has been renamed 'volume' options in r.color, r3.colors and v.colors to raster_3d.

I would suggest do the same with m.nviz.image, any objections?

[martinl]

Replying to hellik:

> stream_rast	Name for output raster map with unique stream ids

changed to stream_raster in r63706

> stream_vect	Name for output vector map with unique stream ids

changed to stream_vector in r63706

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

transmissivitysingleway changed to transmissivity_singleway in r63709

> 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

no opinion here

> start_rast	Name of starting raster points map

changed to start_raster in r63711

> 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

no opinion here

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

changed to slope_tolerance in r63713

> 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

no opinion here

> 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

num_partitions changed to npartitions (to follow other similar options like nwalkers, npoints and others - number of ...)

> ncolumn	Node cost column
> 		Node cost column (number)

This is more significant issue, all of v.net modules uses various shortened options

> 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)

no opinion here

> 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)

see my note about v.net modules above

[martinl]

Replying to martinl:

> > ncolumn	Node cost column
> > 		Node cost column (number)

This is more significant issue, all of v.net modules uses various shortened options

> > 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)

see my note about v.net modules above

I went though v.net modules and renamed

alayer -> arc_layer
nlayer -> node_layer
type -> arc_type
ccats -> center_cats
ncolumn -> node_column
afcolumn -> arc_column
abcolumn -> arc_backward_column
tcats -> terminal_cats
nsp -> npoints
coordinate -> coordinates
vis -> visibility

[martinl]

Replying to martinl:

I went though v.net modules and renamed

see r63730

[cmbarton]

Replying to martinl:

Replying to martinl:

> > ncolumn	Node cost column
> > 		Node cost column (number)

This is more significant issue, all of v.net modules uses various shortened options

> > 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)

see my note about v.net modules above

I went though v.net modules and renamed

alayer -> arc_layer
nlayer -> node_layer
type -> arc_type
ccats -> center_cats
ncolumn -> node_column
afcolumn -> arc_column
abcolumn -> arc_backward_column
tcats -> terminal_cats
nsp -> npoints
coordinate -> coordinates
vis -> visibility

Thanks much. These are a great improvement. IIRC, the vector overlay/query modules could use something similar. Perhaps already done as I've had no time to investigate at the end of semester here. This is a lot of work I realize but greatly appreciated.

Michael

[wenzeslaus]

Replying to neteler:

Replying to wenzeslaus:

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.

Sounds good to me.

Done in r63741. Should be backported soon I guess.

I also extended renamed_options​ with G7:r.his and G7:d.his in r63740 but with d.shade because also the module was renamed.

[martinl]

Replying to wenzeslaus:

Done in r63741. Should be backported soon I guess.

I also extended renamed_options​ with G7:r.his and G7:d.his in r63740 but with d.shade because also the module was renamed.

it seems to be OK to me, please backport.

[glynn]

Replying to neteler:

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?

Yes (although as part of a change which would also remove the ambiguity in that case).

My intention is to modify the code which calls match_option() (set_option() and check_string()) to record which options are matched. If one option is a prefix of all of the others, the string will be considered to unambiguously match that option. If the match is ambiguous, we now have a list of which options matched, which can be used for the error message.

[glynn]

Replying to glynn:

My intention is to modify the code which calls match_option() (set_option() and check_string()) to record which options are matched. If one option is a prefix of all of the others, the string will be considered to unambiguously match that option. If the match is ambiguous, we now have a list of which options matched, which can be used for the error message.

This change is implemented in r63744.

Currently, an ambiguous match for an option key lists the matches, but an ambiguous match for a value (opt->options) doesn't (as the error message is generated by the caller).

[martinl]

Replying to glynn:

This change is implemented in r63744.

Currently, an ambiguous match for an option key lists the matches, but an ambiguous match for a value (opt->options) doesn't (as the error message is generated by the caller).

cool! It open doors to rename options line n,s,e,w to north,south,east,west in g.region. The shorted options will continue to work...

[martinl]

Replying to martinl:

cool! It open doors to rename options line n,s,e,w to north,south,east,west in g.region. The shorted options will continue to work...

sorry, I was wrong, attempt

          north   Value for the northern edge
          south   Value for the southern edge
           east   Value for the eastern edge
           west   Value for the western edge
            top   Value for the top edge
         bottom   Value for the bottom edge
           rows   Number of rows in the new region
           cols   Number of columns in the new region
     resolution   2D grid resolution (north-south and east-west)
  resolution_3d   3D grid resolution (north-south, east-west and top-bottom)
  ns_resolution   North-south 2D grid resolution
  ew_resolution   East-west 2D grid resolution
  tb_resolution   Top-bottom 3D grid resolution

Command

g.region n=1 s=0 e=1 w=0 t=1 b=0 res=1 res3=1 nsres=1 ewres=1 tbres=1

fails with

ERROR: g.region: Sorry, <n=> is ambiguous
ERROR: Option <north=> matches
ERROR: Option <ns_resolution=> matches
ERROR: g.region: Sorry, <s=> is ambiguous
ERROR: Option <south=> matches
ERROR: Option <save=> matches
ERROR: g.region: Sorry, <e=> is ambiguous
ERROR: Option <east=> matches
ERROR: Option <ew_resolution=> matches
ERROR: g.region: Sorry, <t=> is ambiguous
ERROR: Option <top=> matches
ERROR: Option <tb_resolution=> matches

[martinl]

Replying to martinl:

Replying to glynn:

This change is implemented in r63744.

Currently, an ambiguous match for an option key lists the matches, but an ambiguous match for a value (opt->options) doesn't (as the error message is generated by the caller).

cool!

I meant that

g.region rast=elevation

will work, which is very good news.

[wenzeslaus]

Replying to martinl:

Replying to wenzeslaus:

Done in r63741. Should be backported soon I guess.

I also extended renamed_options​ with G7:r.his and G7:d.his in r63740 but with d.shade because also the module was renamed.

it seems to be OK to me, please backport.

Backported in r63755.

[wenzeslaus]

If I understand correctly, we still want to use full option names for GRASS code (and perhaps documentation) and to advise it for user scripts. In this case it would be good to have a way to proof that the full option names are used, something like an environmental variable GRASS_FULL_OPTION_NAMES which would switch off the advanced word/character matching and renamed_options so that only full names would be considered as valid.

[glynn]

Replying to wenzeslaus:

If I understand correctly, we still want to use full option names for GRASS code (and perhaps documentation) and to advise it for user scripts.

Indeed.

The main issues I see with abbreviations are:

1. Searching for use of an option name (grep, Ctrl+F, etc) will miss abbreviations.

2. Abbreviations make code harder to understand for people who aren't fluent in English.

In this case it would be good to have a way to proof that the full option names are used, something like an environmental variable GRASS_FULL_OPTION_NAMES which would switch off the advanced word/character matching and renamed_options so that only full names would be considered as valid.

r63765 generates a warning if GRASS_FULL_OPTION_NAMES is set (to anything) and the found string isn't an exact match for the given string.

[neteler]

I have taken liberty to backport the current parser state in r63772 + r63774.

This including an attempt to document GRASS_FULL_OPTION_NAMES in r63773, please improve.

[wenzeslaus]

Replying to glynn:

Replying to wenzeslaus:

If I understand correctly, we still want to use full option names for GRASS code (and perhaps documentation) and to advise it for user scripts.

Indeed.

The main issues I see with abbreviations are:

1. Searching for use of an option name (grep, Ctrl+F, etc) will miss abbreviations.

2. Abbreviations make code harder to understand for people who aren't fluent in English.

In this case it would be good to have a way to proof that the full option names are used, something like an environmental variable GRASS_FULL_OPTION_NAMES which would switch off the advanced word/character matching and renamed_options so that only full names would be considered as valid.

r63765 generates a warning if GRASS_FULL_OPTION_NAMES is set (to anything) and the found string isn't an exact match for the given string.

Thanks that's good but wouldn't be better to do just the strcmp comparison (to be faster) and fail (with fatal error) which would show these things in tests (errors are clearly visible, warning must be searched for in the output). I think the primary use-case for GRASS_FULL_OPTION_NAMES is when you actually want to find the long options and then it is just more effective to fail. The secondary use-case, I can think of, is making parsing faster which is only possible if this check is alternative, not addition. Do you have different opinion? (This is not necessary to decide before the release I belief because it is not real API considering the use-cases.)

[wenzeslaus]

I improved options for G7:r.ros and G7:r.spread in r63777. The biggest change is actually not the longer names but the change of output which was just unnecessary use of basename concept. I replaced that by 4 new explicit options. (I also improved the naming but not everywhere, so feel free to change it if you have better ideas.)

Although it was already discussed here or in #2136, I submitted some patches and Martin committed something, there might be more changes like this needed. The point is that it is better to avoid basename if possible because explicit options are easier to understand, better for scripting and necessary for applications like QGIS.

[glynn]

Replying to wenzeslaus:

r63765 generates a warning if GRASS_FULL_OPTION_NAMES is set (to anything) and the found string isn't an exact match for the given string.

Thanks that's good but wouldn't be better to do just the strcmp comparison (to be faster) and fail (with fatal error) which would show these things in tests (errors are clearly visible, warning must be searched for in the output). I think the primary use-case for GRASS_FULL_OPTION_NAMES is when you actually want to find the long options and then it is just more effective to fail. The secondary use-case, I can think of, is making parsing faster which is only possible if this check is alternative, not addition. Do you have different opinion? (This is not necessary to decide before the release I belief because it is not real API considering the use-cases.)

I just took the simplest route for now. I don't consider performance issues at this scale to be relevant.

Changing the G_warning() to G_fatal_error() would be trivial, if that's desired.

[wenzeslaus]

Replying to glynn:

Replying to wenzeslaus:

r63765 generates a warning if GRASS_FULL_OPTION_NAMES is set (to anything) and the found string isn't an exact match for the given string.

Thanks that's good but wouldn't be better to do just the strcmp comparison (to be faster) and fail (with fatal error) which would show these things in tests (errors are clearly visible, warning must be searched for in the output). I think the primary use-case for GRASS_FULL_OPTION_NAMES is when you actually want to find the long options and then it is just more effective to fail. The secondary use-case, I can think of, is making parsing faster which is only possible if this check is alternative, not addition. Do you have different opinion? (This is not necessary to decide before the release I belief because it is not real API considering the use-cases.)

I just took the simplest route for now. I don't consider performance issues at this scale to be relevant.

I was hitting some issues when I was calling some modules to do simple task many time, so I'm looking for ways how to decrease the cost of new processes but I don't have any benchmark for the option parsing.

Changing the G_warning() to G_fatal_error() would be trivial, if that's desired.

Not sure about the others but it makes sense to me to switch to G_fatal_error().

[wenzeslaus]

Continuing in what I wrote in comment:17 and following ones changed G7:r.rgb in the way that fixed number of outputs handled by basename was replaced by specific options. This is not only more explicit but also fits more to the context when same rasters are used as input and in case of r.rgb allows to request only some outputs.

Old r.rgb:

Usage:
 r.rgb input=name [output=basename] [--overwrite] [--help] [--verbose]
   [--quiet] [--ui]

Parameters:
   input   Name of input raster map
  output   Name of output basename raster map(s)
            Default: input

New r.rgb:

Usage:
 r.rgb input=name red=name green=name blue=name [--overwrite]
   [--help] [--verbose] [--quiet] [--ui]

Parameters:
  input   Name of input raster map
    red   Red channel raster map name
  green   Green channel raster map name
   blue   Blue channel raster map name

If somebody would like to learn how to write a test, this is a great opportunity. On might do something like:

r.composite r=lsat7_2002_10 g=lsat7_2002_30 b=lsat7_2002_50 o=lsat_comp
r.rgb i=lsat_comp r=lsat7_2002_10 g=lsat7_2002_30 b=lsat7_2002_50

And then test difference of old and new maps. The results might not be identical but should be close to each other. Another tests can check if only one map is created when one map is requested.

I cannot work on this more but similar change as I did for r.rgb could be done for G7:r.blend too. It also has an output option which is a "basename for red, green and blue output raster maps".

There are some other modules I'm not completely sure about such as G7:i.pansharpen and G7:i.topo.corr.

Some other seems that they don't need this change (e.g., G7:i.pca, G7:i.landsat.toar, G7:i.landsat.acca, and G7:i.tasscap) because the number of outputs is variable. However, I'm not sure how the suffixes are generated, sometimes it seems that they are even expected on the input. Standard basename separator (underscore, #2136) should be used in all cases otherwise it is not really standard, now many of them are probably using dot.

Last issue I know about is G7:r.texture where the number of outputs depends on number of requested textures. G7:r.neighbors actually solves this issue by using output not as a basename but as multiple and requesting as many outputs as requested methods.

[neteler]

While checking all QGIS processing files (​https://github.com/qgis/QGIS/tree/master/python/plugins/processing/algs/grass7) I have cleaned up lib/gis/renamed_options (r64056 + r64057, r64062 + r64063).

The following issues are open (TODO):

• r.mapcalculator: outfile -> output? (any implications in the wxGUI?)
• r.param.scape: param -> method?
• r.walk; percent_memory -> memory?

Then here I suggest to update for consistency:

v.lidar.correction
       sce   Interpolation spline step value in east direction
             default: 25
       scn   Interpolation spline step value in north direction
             default: 25
--> ew_step/ns_step (as in v.surf.bspline)

v.lidar.edgedetection
       see   Interpolation spline step value in east direction
             default: 4
       sen   Interpolation spline step value in north direction
             default: 4
--> ew_step/ns_step (as in v.surf.bspline)

No idea if the default values should be the same?

Eventually a potential issue in file "lib/gis/renamed_options":

# r.reclass.area
r.reclass.area|lesser,greater:value, mode
 --> syntax ok?

[cmbarton]

Replying to neteler:

While checking all QGIS processing files (​https://github.com/qgis/QGIS/tree/master/python/plugins/processing/algs/grass7) I have cleaned up lib/gis/renamed_options (r64056 + r64057, r64062 + r64063).

A few thoughts

The following issues are open (TODO):

• r.mapcalculator: outfile -> output? (any implications in the wxGUI?)

Yes. This will break the GUI map calculator. Why the change? Doesn't the map calculator always create a new file?

• r.param.scape: param -> method?
• r.walk; percent_memory -> memory?

These seem OK

Then here I suggest to update for consistency:

v.lidar.correction
       sce   Interpolation spline step value in east direction
             default: 25
       scn   Interpolation spline step value in north direction
             default: 25
--> ew_step/ns_step (as in v.surf.bspline)

v.lidar.edgedetection
       see   Interpolation spline step value in east direction
             default: 4
       sen   Interpolation spline step value in north direction
             default: 4
--> ew_step/ns_step (as in v.surf.bspline)

These are definitely an improvement

No idea if the default values should be the same?

If this works like v.surf.bspline, the defaults are almost always wrong. Some multiple of the current resolution (2X or 10X) would be better than the current default of 4. It is much better for this number to be larger than optimum than smaller than optimum.

v.surf.bspline has an option to calculate a reasonable spline step that runs very quickly. I assume that this is the same algorithm found in the lidar processing modules. A note suggesting using this option would be good to add to the spline step description for all modules.

Michael

}}}

Eventually a potential issue in file "lib/gis/renamed_options":

# r.reclass.area
r.reclass.area|lesser,greater:value, mode
 --> syntax ok?

[annakrat]

Replying to neteler:

The following issues are open (TODO):

• r.mapcalculator: outfile -> output? (any implications in the wxGUI?)

what is r.mapcalculator?

• r.param.scape: param -> method?
• r.walk; percent_memory -> memory?

+1

Eventually a potential issue in file "lib/gis/renamed_options":

# r.reclass.area
r.reclass.area|lesser,greater:value, mode
 --> syntax ok?

I would remove it from there, because the logic of the parameters changed so you cannot easily map the old name to a new name.

[neteler]

Replying to cmbarton:

Replying to neteler:

• r.mapcalculator: outfile -> output? (any implications in the wxGUI?)

Yes. This will break the GUI map calculator. Why the change? Doesn't the map calculator always create a new file?

I propose the change for consistency. Isn't the result a map rather than a file (we use that for eg ASCII file output)? So the parameter name should be "output" then.

Replying to annakrat:

Replying to neteler:

The following issues are open (TODO):

• r.mapcalculator: outfile -> output? (any implications in the wxGUI?)

what is r.mapcalculator?

No idea, I found it in the QGIS processing scripts being used. In GRASS, it is here: scripts/r.mapcalculator/r.mapcalculator

[wenzeslaus]

Replying to neteler:

Replying to cmbarton:

Replying to neteler:

• r.mapcalculator: outfile -> output? (any implications in the wxGUI?)

Yes. This will break the GUI map calculator.

Perhaps some Tcl/Tk one, I don't know. wxGUI raster map calculator is using r.mapcalc. r.mapcalculator is not used anywhere in GRASS GIS according to my knowledge and grep search.

Why the change? Doesn't the map calculator always create a new file?

I propose the change for consistency. Isn't the result a map rather than a file (we use that for eg ASCII file output)? So the parameter name should be "output" then.

It should be changed to "output". Any option which is name of some kind of output should be named "output" by default. Reason to name it different would be for example more than one option which is a name of something on output. Also, we are not using "file" to "denote raster map" (for various reasons).

Replying to annakrat:

Replying to neteler:

The following issues are open (TODO):

• r.mapcalculator: outfile -> output? (any implications in the wxGUI?)

what is r.mapcalculator?

No idea, I found it in the QGIS processing scripts being used. In GRASS, it is here: scripts/r.mapcalculator/r.mapcalculator

QGIS processing needs to use r.mapcalculator instead of r.mapcalc because it needs to convert input maps and output maps to and from GRASS GIS but in case of r.mapcalc, map names are hidden in the expression. r.mapcalculator on the other hand, has several options which are map names and output is not part of the expression but also an option as it would be for other modules.

Similar issues applies to more modules as I posted here or to the basename/prefix ticket. I recently fixed r.ros. I learned about these issues and r.mapcalculator in ​GRASS-dev QGIS Processing & GRASS.

r.mapcalculator may create some confusion because is not part of standard GRASS GIS distribution (compilation switched off by default). It is also not in the documentation. I don't know if the confusion is also on the QGIS site. Another issue is that it is in Shell, not Python, so it should not work in GRASS 7 on MS Windows as far as I understand. Finally, so revision of messages is needed, error message "Calculating $GIS_OPT_OUTFILE. Try expert mode." shown after r.mapcalc failed just caught my eye.

[cmbarton]

Indeed. I was confused. I assumed that r.mapcalculator was a renaming of r.mapcalc.

[glynn]

Replying to wenzeslaus:

r.mapcalculator may create some confusion because is not part of standard GRASS GIS distribution (compilation switched off by default). It is also not in the documentation. I don't know if the confusion is also on the QGIS site. Another issue is that it is in Shell, not Python, so it should not work in GRASS 7 on MS Windows as far as I understand. Finally, so revision of messages is needed, error message "Calculating $GIS_OPT_OUTFILE. Try expert mode." shown after r.mapcalc failed just caught my eye.

r.mapcalculator was never converted to Python. It was deemed unnecessary now that r.mapcalc uses G_parser(). r.mapcalculator was created when r.mapcalc didn't use G_parser() which meant that it couldn't generate a GUI option dialog.

If there's some situation where it's useful, it should be converted to Python. Otherwise, it should be removed from the 7.0 tree.

[wenzeslaus]

Replying to glynn:

Replying to wenzeslaus:

r.mapcalculator may create some confusion because is not part of standard GRASS GIS distribution (compilation switched off by default). It is also not in the documentation. I don't know if the confusion is also on the QGIS site. Another issue is that it is in Shell, not Python, so it should not work in GRASS 7 on MS Windows as far as I understand. Finally, so revision of messages is needed, error message "Calculating $GIS_OPT_OUTFILE. Try expert mode." shown after r.mapcalc failed just caught my eye.

r.mapcalculator was never converted to Python. It was deemed unnecessary now that r.mapcalc uses G_parser(). r.mapcalculator was created when r.mapcalc didn't use G_parser() which meant that it couldn't generate a GUI option dialog.

If there's some situation where it's useful, it should be converted to Python. Otherwise, it should be removed from the 7.0 tree.

I think it is needed for QGIS and perhaps other things wrapping GRASS modules. Although I don't know much, the issue appears to be that they don't know what maps needs to be prepared. I've never really studied the problem. There might be some other way for callers to deal with the expression. For example, temporal framework also wraps r.mapcalc and its quite successful. Or r.mapcalc or some module derived from it (similarly as r3.mapcalc is) can just parse the expression and output the names in some key-value/parseable form, or there can be a Python function for it. Some inside from application developers using GRASS GIS would be helpful.

[mlennert]

Replying to glynn:

Replying to wenzeslaus:

r.mapcalculator may create some confusion because is not part of standard GRASS GIS distribution (compilation switched off by default). It is also not in the documentation. I don't know if the confusion is also on the QGIS site. Another issue is that it is in Shell, not Python, so it should not work in GRASS 7 on MS Windows as far as I understand. Finally, so revision of messages is needed, error message "Calculating $GIS_OPT_OUTFILE. Try expert mode." shown after r.mapcalc failed just caught my eye.

r.mapcalculator was never converted to Python. It was deemed unnecessary now that r.mapcalc uses G_parser(). r.mapcalculator was created when r.mapcalc didn't use G_parser() which meant that it couldn't generate a GUI option dialog.

If there's some situation where it's useful, it should be converted to Python. Otherwise, it should be removed from the 7.0 tree.

+1 for removal. If there are any issues with using the grass7 r.mapcalc in other software (e.g. QGIS) then let's try to solve these issues.

Moritz

[neteler]

Replying to neteler:

• r.param.scale: param -> method?
• r.walk; percent_memory -> memory?

Done in r64088 + r64089.

v.lidar.correction
       sce   Interpolation spline step value in east direction
             default: 25
       scn   Interpolation spline step value in north direction
             default: 25
--> ew_step/ns_step (as in v.surf.bspline)

v.lidar.edgedetection
       see   Interpolation spline step value in east direction
             default: 4
       sen   Interpolation spline step value in north direction
             default: 4
--> ew_step/ns_step (as in v.surf.bspline)

No idea if the default values should be the same?

Parameters renamed in r64083/r64085 + r64084/r64086. The default values I have still left as before.

TODO: r.mapcalculator, r.reclass.area entry in lib/gis/renamed_option

[martinl]

Replying to neteler:

Parameters renamed in r64083/r64085 + r64084/r64086. The default values I have still left as before.

TODO: r.mapcalculator, r.reclass.area entry in lib/gis/renamed_option

Is it still open?

[neteler]

Replying to martinl:

Replying to neteler:

Parameters renamed in r64083/r64085 + r64084/r64086. The default values I have still left as before.

TODO: r.mapcalculator, r.reclass.area entry in lib/gis/renamed_option

Is it still open?

r.reclass.area is done.

r.mapcalculator was proposed to be removed if not needed by QGIS Processing. I wonder how harmful it is to just keep it.

[wenzeslaus]

Replying to neteler:

r.mapcalculator was proposed to be removed if not needed by QGIS Processing. I wonder how harmful it is to just keep it.

r.mapcalculator documentation should say that it is only for QGIS and it should not be used by others because it is not guaranteed.

I have very limited QGIS Processing knowledge but I guess QGIS Processing still needs r.mapcalculator because there is not way how QGIS Processing can know which maps to import and which maps to export. From my understanding, the only way to know would be if standard r.mapcalc would offer a flag (e.g. -l) which lists the inputs and output(s?):

> r.mapcalc -l "test1 = test2 + test3 + 5"
input=test2,test3
output=test1

and does only the parsing, not the actual computation. In GRASS GIS, this could be used for example, by GUI to validate the expression. Instead of flag, a separate module can be used (as mentioned in comment:190).

I'm not sure how serious it is but as mentioned in comment:187, r.mapcalculator is a Shell script and Shell script are no longer supported by standalone GRASS GIS. But perhaps it is not an issue because QGIS installs its own GRASS GIS on MS Windows, OSGeo4W probably contains some shell and all systems except for MS Windows just have some shell.

[martinl]

Replying to wenzeslaus:

I'm not sure how serious it is but as mentioned in comment:187, r.mapcalculator is a Shell script and Shell script are no longer supported by standalone GRASS GIS. But perhaps it is not

right, it's a Shell script and currently disabled in Makefile. So let's remove it from trunk and relbr70. Any objection?

an issue because QGIS installs its own GRASS GIS on MS Windows, OSGeo4W probably contains some shell and all systems except for MS Windows just have some shell.

I needed anyone can rewrite r.mapcalculator to Python and publish as Addons.

[wenzeslaus]

Replying to martinl:

Replying to wenzeslaus:

I'm not sure how serious it is but as mentioned in comment:187, r.mapcalculator is a Shell script and Shell script are no longer supported by standalone GRASS GIS. But perhaps it is not

right, it's a Shell script and currently disabled in Makefile. So let's remove it from trunk and relbr70. Any objection?

This will break QGIS Processing grass7:r.mapcalculator algorithm:

​https://github.com/qgis/QGIS/blob/master/python/plugins/processing/algs/grass7/description/r.mapcalculator.txt

If expect that QGIS Processing will not work with 7.0.0 and that this is a feature for 7.0.1 or later, then it is fine I guess.

an issue because QGIS installs its own GRASS GIS on MS Windows, OSGeo4W probably contains some shell and all systems except for MS Windows just have some shell.

I needed anyone can rewrite r.mapcalculator to Python and publish as Addons.

This sounds good. QGIS "just" has to ensure that Addons will be installed (QGIS Processing currently does not support Addons, only things from core which are ​explicitly listed).

[martinl]

Replying to wenzeslaus:

If expect that QGIS Processing will not work with 7.0.0 and that this is a feature for 7.0.1 or later, then it is fine I guess.

r.mapcalculator and other remaining shell scripts removed in r64697 and backported in r64700.

[glynn]

Replying to wenzeslaus:

I have very limited QGIS Processing knowledge but I guess QGIS Processing still needs r.mapcalculator because there is not way how QGIS Processing can know which maps to import and which maps to export. From my understanding, the only way to know would be if standard r.mapcalc would offer a flag (e.g. -l) which lists the inputs and output(s?):

Such an option would be fairly simple to add. Most of the relevant logic is already present in execute() in r.mapcalc/evaluate.c.

The initialize() function populates the list of input maps (in r.mapcalc/map.c or map3.c), which can be used to generate the list of inputs. Each top-level assignment expression defines an output.

[martinl]

It seems to me that there is no open issue related to this ticket. I am taking liberty to close the ticket, feel free to reopen if needed.

[martinl]

Replying to glynn:

Replying to wenzeslaus:

I have very limited QGIS Processing knowledge but I guess QGIS Processing still needs r.mapcalculator because there is not way how QGIS Processing can know which maps to import and which maps to export. From my understanding, the only way to know would be if standard r.mapcalc would offer a flag (e.g. -l) which lists the inputs and output(s?):

Such an option would be fairly simple to add. Most of the relevant logic is already present in execute() in r.mapcalc/evaluate.c.

The initialize() function populates the list of input maps (in r.mapcalc/map.c or map3.c), which can be used to generate the list of inputs. Each top-level assignment expression defines an output.

reported as #2592