[GRASS-dev] RFC: Module Option Parser

Brad_Douglas · December 15, 2006, 3:28am

I'm in the process of updating/upgrading the imagery libraries and
modules. I have run into a problem which I wish to simplify, but would
require modification of lib/gis/parser.c, etc.

I think we can all agree that i.ortho.photo is a mess and hasn't been
able to evolve as the rest of GRASS has. I want to remove as much
interactivity as possible, but that creates a dilemma:

A) I can break i.ortho.photo into 7-8 separate modules, or
B) I can make it a single module by using the following structure:

Option A does not set well with me and Markus has agreed, privately, due
to the ever growing list of GRASS modules, so I need something a bit
'better'.

- Create an Option that allows to select from a series of commands like
v.build does:

    tool_opt = G_define_option ();
    tool_opt->key = "tool";
    tool_opt->type = TYPE_STRING;
    tool_opt->required = YES;
    tool_opt->multiple = NO;
    tool_opt->description = _("The ortho tool to use:\n"
                "\t\ttarget - Target imagery location and mapset\n"
                "\t\tdem - Digital Elevation Model (elevation)\n"
                "\t\tcamera - Camera parameters\n"
                "\t\ttransform - Transformation parameters\n"
                "\t\texposure - Exposure parameters\n"
                "\t\tparams - Ortho photo parameters\n"
                "\t\trectify - Rectify the ortho photo");
    tool_opt->options =
"target,dem,camera,transform,exposure,params,rectify";

...but alter it a bit, namely changing the first line to:
tool_opt = G_define_selection(), a new function.

In order to make this work and keep the many options from spamming
i.ortho.photo --help (as many of the above selections have multiple
options that are not related to each other), I would like to propose the
following changes:

- Create a new function: G_define_selection ();
- Add another option to G_define_option () that would link an option to
a selection:
    dem_opt = G_define_option ();
    dem_opt->key = "dem";
    dem_opt->type = TYPE_STRING;
    dem_opt->required = NO;
    dem_opt->multiple = NO;
    dem_opt->description = _("Digital Elevation Model");
--> dem_opt->selection = "dem"; <--- new option

So...when I do 'i.ortho.photo --help', I get a list of selections to
choose from. When I do, 'i.ortho.photo select=dem --help', I only get a
list of options associated with the "dem" selection (ie. dem_opt).

Comments, suggestions, and better ideas are always welcome.

--
Brad Douglas <rez touchofmadness com> KB8UYR/6
Address: 37.493,-121.924 / WGS84 National Map Corps #TNMC-3785

Glynn_Clements1 · December 15, 2006, 9:22am

Brad Douglas wrote:

I'm in the process of updating/upgrading the imagery libraries and
modules. I have run into a problem which I wish to simplify, but would
require modification of lib/gis/parser.c, etc.

I think we can all agree that i.ortho.photo is a mess and hasn't been
able to evolve as the rest of GRASS has. I want to remove as much
interactivity as possible, but that creates a dilemma:

A) I can break i.ortho.photo into 7-8 separate modules, or
B) I can make it a single module by using the following structure:

I strongly favour option A; see below.

Option A does not set well with me and Markus has agreed, privately, due
to the ever growing list of GRASS modules, so I need something a bit
'better'.

- Create an Option that allows to select from a series of commands like
v.build does:

    tool_opt = G_define_option ();
    tool_opt->key = "tool";
    tool_opt->type = TYPE_STRING;
    tool_opt->required = YES;
    tool_opt->multiple = NO;
    tool_opt->description = _("The ortho tool to use:\n"
                "\t\ttarget - Target imagery location and mapset\n"
                "\t\tdem - Digital Elevation Model (elevation)\n"
                "\t\tcamera - Camera parameters\n"
                "\t\ttransform - Transformation parameters\n"
                "\t\texposure - Exposure parameters\n"
                "\t\tparams - Ortho photo parameters\n"
                "\t\trectify - Rectify the ortho photo");
    tool_opt->options =
"target,dem,camera,transform,exposure,params,rectify";

...but alter it a bit, namely changing the first line to:
tool_opt = G_define_selection(), a new function.

In order to make this work and keep the many options from spamming
i.ortho.photo --help (as many of the above selections have multiple
options that are not related to each other), I would like to propose the
following changes:

- Create a new function: G_define_selection ();
- Add another option to G_define_option () that would link an option to
a selection:
    dem_opt = G_define_option ();
    dem_opt->key = "dem";
    dem_opt->type = TYPE_STRING;
    dem_opt->required = NO;
    dem_opt->multiple = NO;
    dem_opt->description = _("Digital Elevation Model");
--> dem_opt->selection = "dem"; <--- new option

So...when I do 'i.ortho.photo --help', I get a list of selections to
choose from. When I do, 'i.ortho.photo select=dem --help', I only get a
list of options associated with the "dem" selection (ie. dem_opt).

Comments, suggestions, and better ideas are always welcome.

Extending the infrastructure for a single module is normally a clue
that the problem is with the module.

If most of the options only apply to a specific "mode", that suggests
that each mode should be implemented as a separate module. If they
share common code, make it a library.

--
Glynn Clements <glynn@gclements.plus.com>

Brad_Douglas · December 19, 2006, 9:50pm

On Fri, 2006-12-15 at 09:22 +0000, Glynn Clements wrote:

Brad Douglas wrote:

> I'm in the process of updating/upgrading the imagery libraries and
> modules. I have run into a problem which I wish to simplify, but would
> require modification of lib/gis/parser.c, etc.
>
> I think we can all agree that i.ortho.photo is a mess and hasn't been
> able to evolve as the rest of GRASS has. I want to remove as much
> interactivity as possible, but that creates a dilemma:
>
> A) I can break i.ortho.photo into 7-8 separate modules, or
> B) I can make it a single module by using the following structure:

I strongly favour option A; see below.

> Option A does not set well with me and Markus has agreed, privately, due
> to the ever growing list of GRASS modules, so I need something a bit
> 'better'.
>
> - Create an Option that allows to select from a series of commands like
> v.build does:
>
> tool_opt = G_define_option ();
> tool_opt->key = "tool";
> tool_opt->type = TYPE_STRING;
> tool_opt->required = YES;
> tool_opt->multiple = NO;
> tool_opt->description = _("The ortho tool to use:\n"
> "\t\ttarget - Target imagery location and mapset\n"
> "\t\tdem - Digital Elevation Model (elevation)\n"
> "\t\tcamera - Camera parameters\n"
> "\t\ttransform - Transformation parameters\n"
> "\t\texposure - Exposure parameters\n"
> "\t\tparams - Ortho photo parameters\n"
> "\t\trectify - Rectify the ortho photo");
> tool_opt->options =
> "target,dem,camera,transform,exposure,params,rectify";
>
> ...but alter it a bit, namely changing the first line to:
> tool_opt = G_define_selection(), a new function.
>
> In order to make this work and keep the many options from spamming
> i.ortho.photo --help (as many of the above selections have multiple
> options that are not related to each other), I would like to propose the
> following changes:
>
> - Create a new function: G_define_selection ();
> - Add another option to G_define_option () that would link an option to
> a selection:
> dem_opt = G_define_option ();
> dem_opt->key = "dem";
> dem_opt->type = TYPE_STRING;
> dem_opt->required = NO;
> dem_opt->multiple = NO;
> dem_opt->description = _("Digital Elevation Model");
> --> dem_opt->selection = "dem"; <--- new option
>
> So...when I do 'i.ortho.photo --help', I get a list of selections to
> choose from. When I do, 'i.ortho.photo select=dem --help', I only get a
> list of options associated with the "dem" selection (ie. dem_opt).
>
> Comments, suggestions, and better ideas are always welcome.

Extending the infrastructure for a single module is normally a clue
that the problem is with the module.

I may have had i.ortho.photo in mind, but I intentionally tried to make
it extensible to other modules.

Modules that could readily benefit (by no means a complete list):
v.distance
v.lidar*
v.lrs*
v.net*
r.resamp*
v.surf*
r.li*
r.le*
r.support*

If most of the options only apply to a specific "mode", that suggests
that each mode should be implemented as a separate module. If they
share common code, make it a library.

I disagree. Currently there are about 150 raster and 65 vector modules.
The list is not shrinking and becoming a bit unwieldy.

Either way, I don't want to waste a whole lot of time discussing this.
I made my best case. If option A still looks like the best route, then
that's what I'll do.

--
Brad Douglas <rez touchofmadness com> KB8UYR/6
Address: 37.493,-121.924 / WGS84 National Map Corps #TNMC-3785

Glynn_Clements1 · December 21, 2006, 5:50pm

Brad Douglas wrote:

> > In order to make this work and keep the many options from spamming
> > i.ortho.photo --help (as many of the above selections have multiple
> > options that are not related to each other), I would like to propose the
> > following changes:
> >
> > - Create a new function: G_define_selection ();
> > - Add another option to G_define_option () that would link an option to
> > a selection:
> > dem_opt = G_define_option ();
> > dem_opt->key = "dem";
> > dem_opt->type = TYPE_STRING;
> > dem_opt->required = NO;
> > dem_opt->multiple = NO;
> > dem_opt->description = _("Digital Elevation Model");
> > --> dem_opt->selection = "dem"; <--- new option
> >
> > So...when I do 'i.ortho.photo --help', I get a list of selections to
> > choose from. When I do, 'i.ortho.photo select=dem --help', I only get a
> > list of options associated with the "dem" selection (ie. dem_opt).
> >
> > Comments, suggestions, and better ideas are always welcome.
>
> Extending the infrastructure for a single module is normally a clue
> that the problem is with the module.

I may have had i.ortho.photo in mind, but I intentionally tried to make
it extensible to other modules.

Modules that could readily benefit (by no means a complete list):
v.distance
v.lidar*
v.lrs*
v.net*
r.resamp*
v.surf*
r.li*
r.le*
r.support*

> If most of the options only apply to a specific "mode", that suggests
> that each mode should be implemented as a separate module. If they
> share common code, make it a library.

I disagree. Currently there are about 150 raster and 65 vector modules.
The list is not shrinking and becoming a bit unwieldy.

I don't see a problem with having a large number of modules.

From a developer's perspective, many small modules are easier to deal

with than one large module, as you have to analyse less code to
understand the consequences of making any given change.

I view modules as being "functions" within an API, rather than as
applications within a package or suite.

Having distinct "modes" within a single module should only be used
when the difference in code is small and most options apply to most
modes.

If the amount of code in common and the amount of code specific to
each mode are both substantial, I would prefer multiple modules with
the common code in a (private) library.

A good example of something which shouldn't be a single module is
r.support. Modifying the category list and modifying the colour table
have almost nothing in common.

Similarly, the various r.resamp* modules all use fundamentally
different algorithms and have different parameters (r.resamp.rst has
many parameters, most of which aren't applicable to other algorithms).
A combined resampling module would be nothing more than a front-end to
what are essentially several entirely different modules.

--
Glynn Clements <glynn@gclements.plus.com>

Markus_Neteler1 · December 22, 2006, 8:36am

Glynn Clements wrote on 12/21/2006 06:50 PM:

Brad Douglas wrote:

If most of the options only apply to a specific "mode", that suggests
that each mode should be implemented as a separate module. If they
share common code, make it a library.

I disagree. Currently there are about 150 raster and 65 vector modules.
The list is not shrinking and becoming a bit unwieldy.

I don't see a problem with having a large number of modules.

From a user's perspective this can become very confusing.

I tried to introduce the "keywords" concept to make the user easier
navigate through the available algorithms, but didn't get much
feedback on this.

ls -l $HOME/grass63/dist.x86_64-unknown-linux-gnu/bin | wc -l
321

plus scripts is already too much in my opinion.

Markus

Florian_Kindl · December 22, 2006, 9:11am

On Dec 22 [ 9:36], Markus Neteler wrote:

Glynn Clements wrote on 12/21/2006 06:50 PM:
> Brad Douglas wrote:
>
>
>>> If most of the options only apply to a specific "mode", that suggests
>>> that each mode should be implemented as a separate module. If they
>>> share common code, make it a library.
>>>
>> I disagree. Currently there are about 150 raster and 65 vector modules.
>> The list is not shrinking and becoming a bit unwieldy.
>>
>
> I don't see a problem with having a large number of modules.
>
>
>From a user's perspective this can become very confusing.
I tried to introduce the "keywords" concept to make the user easier
navigate through the available algorithms, but didn't get much
feedback on this.

ls -l $HOME/grass63/dist.x86_64-unknown-linux-gnu/bin | wc -l
321

plus scripts is already too much in my opinion.

The pain is eased by the hierarchical naming conventions, i.e. r.*,
r.li.*, v.net.* and so on. However, it is paramount to adhere to those
naming conventions. If the modules do have descriptive names arranged in a
meaningful hierarchy it's not all that hard to find what you're looking
for.

my 2 euro-cent,
\flo.

--
Florian Kindl
Institute of Geography
University of Innsbruck

Daniel_Calvelo · December 23, 2006, 3:43pm

On 12/22/06, Florian Kindl <florian.kindl@uibk.ac.at> wrote:

On Dec 22 [ 9:36], Markus Neteler wrote:
> Glynn Clements wrote on 12/21/2006 06:50 PM:
> > Brad Douglas wrote:
> >
> >>> If most of the options only apply to a specific "mode", that suggests
> >>> that each mode should be implemented as a separate module. If they
> >>> share common code, make it a library.
> >>>
> >> I disagree. Currently there are about 150 raster and 65 vector modules.
> >> The list is not shrinking and becoming a bit unwieldy.
> >>
> >
> > I don't see a problem with having a large number of modules.
> >
> >From a user's perspective this can become very confusing.
> I tried to introduce the "keywords" concept to make the user easier
> navigate through the available algorithms, but didn't get much
> feedback on this.
>
> ls -l $HOME/grass63/dist.x86_64-unknown-linux-gnu/bin | wc -l
> 321
>
> plus scripts is already too much in my opinion.
>

The pain is eased by the hierarchical naming conventions, i.e. r.*,
r.li.*, v.net.* and so on. However, it is paramount to adhere to those
naming conventions. If the modules do have descriptive names arranged in a
meaningful hierarchy it's not all that hard to find what you're looking
for.

I agree.

From a users perspective, there isn´t much difference between

i.ortho.photo cmd=foo

and

i.ortho.photo.foo

If one of the readline completion thingies is included in the
distribution by default, the latter is tab-searchable and respecting
the "namespaces" introduced by the naming convention is enough to
grant CLI users a good expereience.

As for docs, it could be good to have a main "introductory" page for
the general intent of each command from the "namespace" (in the same
spirit as sql.html and the like) referencing each commands specific
documentation (and linking back). I agree this is a little cumbersome
for maintenance.

As for GUI, I'm not qualified to commente, but my guess is that the
new option-depending-on-option proposal needs some adjustments/
refactoring for the GUI code.

As for code, well, we're talking about using (almost) the same
codebase with several commands in the PGM var of the Makefile, and
shred linking to core functions if any. Of course duplicated code is
to be avoided, but my guess is that librarizing the common functions
and using static linking (I would reserve dynamic for functions
reusable across several modules), there wouldn't be much difference.
Further, disentangling code that is meant to cover several special
cases usually simplifies it, which helps maintenability.

My PEN0.005 (about EUR0.02)

Daniel.

--
-- Daniel Calvelo Aros