[GRASS-dev] [GRASS GIS] #2338: r.horizon rename parameters

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: normal | Milestone: 7.1.0
Component: Default | Version: svn-trunk
Keywords: | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------
What do you think if we rename the options from:
   - elev_in => elevation (like in slope_aspect);
   - horizon_step => step;
   - horizon_start => start;
   - horizon_end => end;
   - horizon => prefix or output_prefix

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

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: critical | Milestone: 7.0.0
Component: Default | Version: svn-trunk
Keywords: | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------
Changes (by martinl):

  * priority: normal => critical
  * milestone: 7.1.0 => 7.0.0

Comment:

Strong +1, for `elevation` should be used standardized option
`G_OPT_R_ELEV`. Milestone moved to 7.0 (renaming options should be done
before releasing 7.0), see wiki:Grass7/NewFeatures#Removedmodules

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

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------
Changes (by hcho):

  * keywords: => r.horizon
  * component: Default => Raster

Comment:

+1. I prefer prefix over output_prefix because it's shorter.

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

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by martinl):

Replying to [comment:2 hcho]:
> +1. I prefer prefix over output_prefix because it's shorter.

note that you can shorten option as you want (keeping unique names), so
'out', 'output' or 'o_p' will work. `output_prefix` seems to be probably
more informative... Also keeping consistency with other modules would be
nice (probably good to have standardized option for modules which produce
maps with given prefix/postfix?)

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

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by hcho):

Oh! I never knew we could use o_p.

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

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by wenzeslaus):

Please read #2136 and look how many modules are using particular names.

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

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by zarch):

Replying to [comment:5 wenzeslaus]:
> Please read #2136 and look how many modules are using particular names.

So I change: horizon => basename, ok?

I wrote two functions, one that return the number of decimals in a string,
and another one that return a format string given the basename, the number
of digits and decimals that we want to use.

I attached the code with two functions plus some tests in main() [0].

Considering that these functionalities are shared between modules I think
we should put them somewhere in the GARSS C API...

Where should we put them? Any ideas?

Pietro

[0] http://trac.osgeo.org/grass/attachment/ticket/2338/main.c

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

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by neteler):

Replying to [comment:3 martinl]:
> Replying to [comment:2 hcho]:
> > +1. I prefer prefix over output_prefix because it's shorter.

Yes, "prefix=" like in r.texture. At least standardize it:

> Also keeping consistency with other modules would be nice
> (probably good to have standardized option for modules which
> produce maps with given prefix/postfix?)

Yes, that should go into the parser.

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

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by martinl):

Replying to [comment:7 neteler]:
> Replying to [comment:3 martinl]:
> > Replying to [comment:2 hcho]:
> > > +1. I prefer prefix over output_prefix because it's shorter.
>
> Yes, "prefix=" like in r.texture. At least standardize it:

Some modules are using `basename` I guess. We should standardize it before
releasing 7.0...

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

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by wenzeslaus):

Replying to [comment:7 neteler]:
> Replying to [comment:3 martinl]:
> > Replying to [comment:2 hcho]:
> > > +1. I prefer prefix over output_prefix because it's shorter.
>
> Yes, "prefix=" like in r.texture. At least standardize it:
>

As I was saying in in #2136: I don't like "prefix" because it is not a
prefix. In my point of view, we are suffixing the analyses name or state
description to the given string. I prefer "basename" because it is what it
is.

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

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by zarch):

Replying to [comment:9 wenzeslaus]:
> Replying to [comment:7 neteler]:
> > Yes, "prefix=" like in r.texture. At least standardize it:
>
>
> As I was saying in in #2136: I don't like "prefix" because
> it is not a prefix. In my point of view, we are suffixing
> the analyses name or state description to the given string.
> I prefer "basename" because it is what it is.

Ok, I used basename and, to avoid repetitions, I added
([http://trac.osgeo.org/grass/changeset/60894 r60894]) two new standard
options:
    - G_OPT_R_BASENAME_INPUT
    - G_OPT_R_BASENAME_OUTPUT
so If in the future we decide to change again from basename to prefix or
something else, we should be able to do this modifying only in a single
place.

But I realize that maybe is not enough. I modify the r.sun module to
handle the changed name in r.horizon, so I used G_OPT_R_BASENAME_INPUT, in
this way in r.sun the "horizon" parameter became "basename", instead I
think that would be clearer to use "horizon_basename".

So what should I do? just leave "basename" or use "horizon_basename"?

Any thoughts on this?

Also in [http://grass.osgeo.org/grass70/manuals/r.sun.html r.sun] probably
we should change:

    - elev_in => elevation
    - asp_in => aspect
    - aspect => ??? aspect_val? vaspect? val_aspect?
    - slope_in => slope
    - slope = ??? slope_val? vslope? val_slope?
    - linke_in => ??? linke
    - lat_in => ??? lat
    - long_in => ??? long
    - horizon => ??? basename or horizon_basename?

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

#2338: r.horizon rename parameters
-------------------------+--------------------------------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: enhancement | Status: new
Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.horizon | Platform: All
      Cpu: All |
-------------------------+--------------------------------------------------

Comment(by annakrat):

Replying to [comment:10 zarch]:
> Replying to [comment:9 wenzeslaus]:
> > Replying to [comment:7 neteler]:
> > > Yes, "prefix=" like in r.texture. At least standardize it:
> >
> >
> > As I was saying in in #2136: I don't like "prefix" because
> > it is not a prefix. In my point of view, we are suffixing
> > the analyses name or state description to the given string.
> > I prefer "basename" because it is what it is.
>
>
> Ok, I used basename and, to avoid repetitions, I added
([http://trac.osgeo.org/grass/changeset/60894 r60894]) two new standard
options:
> - G_OPT_R_BASENAME_INPUT
> - G_OPT_R_BASENAME_OUTPUT
> so If in the future we decide to change again from basename to prefix or
something else, we should be able to do this modifying only in a single
place.

There should be probably also vector version but I am aware of only one
module which could use that (r.sim.water). So that's not a big issue.

>
> But I realize that maybe is not enough. I modify the r.sun module to
handle the changed name in r.horizon, so I used G_OPT_R_BASENAME_INPUT, in
this way in r.sun the "horizon" parameter became "basename", instead I
think that would be clearer to use "horizon_basename".
>
> So what should I do? just leave "basename" or use "horizon_basename"?

horizon_basename I would say

>
> Any thoughts on this?
>
> Also in [http://grass.osgeo.org/grass70/manuals/r.sun.html r.sun]
probably we should change:
>
> - elev_in => elevation
> - asp_in => aspect
> - aspect => ??? aspect_val? vaspect? val_aspect?
aspect_value (the same is already in r.sim.water)
> - slope_in => slope
> - slope = ??? slope_val? vslope? val_slope?
slope_value
> - linke_in => ??? linke
> - lat_in => ??? lat
> - long_in => ??? long
the rest as you suggested

With the new standard option, do you think changing gisprompt would be a
good idea? It could be

{{{
Opt->gisprompt = "old,cell,basename";
}}}

GUI could use that instead of guessing from the parameter name. The widget
could be the standard map selection widget but when selecting the map, it
could try to cut the suffix (in case we have the standardized delimiter
such as `__`). When new series of maps are created, they could be all
added (up to some number).

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

#2338: r.horizon rename parameters
--------------------------+-------------------------------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: enhancement | Status: closed
  Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Resolution: fixed | Keywords: r.horizon
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Changes (by zarch):

  * status: new => closed
  * resolution: => fixed

Comment:

Replying to [comment:11 annakrat]:
> the rest as you suggested

Done in [http://trac.osgeo.org/grass/changeset/61096 r61096].

> With the new standard option, do you think
> changing gisprompt would be a good idea? It could be
>
> {{{
> Opt->gisprompt = "old,cell,basename";
> }}}
>
> GUI could use that instead of guessing from the parameter name.
> The widget could be the standard map selection widget but when
> selecting the map, it could try to cut the suffix (in case we
> have the standardized delimiter such as `__`). When new series
> of maps are created, they could be all added (up to some number).

+1

yes I think that add a basename option to gisprompt could be a good idea.

I close this ticket, we could discuss further about this in
[https://trac.osgeo.org/grass/ticket/2136 #2136].

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

#2338: r.horizon rename parameters
--------------------------+-------------------------------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: enhancement | Status: reopened
  Priority: critical | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: r.horizon, r.sun
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Changes (by neteler):

  * keywords: r.horizon => r.horizon, r.sun
  * status: closed => reopened
  * resolution: fixed =>

Comment:

Reopening as r61098 + r61099 are not yet backported to relbr7 (parameter
name changes)

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

#2338: r.horizon rename parameters
--------------------------+-------------------------------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: enhancement | Status: reopened
  Priority: blocker | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Resolution: | Keywords: r.horizon, r.sun
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Changes (by neteler):

  * priority: critical => blocker

Comment:

TODO: Besides backport check also manual page examples

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

#2338: r.horizon rename parameters
--------------------------+-------------------------------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: enhancement | Status: closed
  Priority: blocker | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Resolution: fixed | Keywords: r.horizon, r.sun
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------
Changes (by neteler):

  * status: reopened => closed
  * resolution: => fixed

Comment:

r.horizon and r.sun parameter changes:

Backported r61096 + r61098 + r61099 in r62430, Closing.

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

#2338: r.horizon rename parameters
--------------------------+-------------------------------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: enhancement | Status: closed
  Priority: blocker | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Resolution: fixed | Keywords: r.horizon, r.sun
  Platform: All | Cpu: All
--------------------------+-------------------------------------------------

Comment(by neteler):

Replying to [comment:15 neteler]:
> r.horizon and r.sun parameter changes:
>
> Backported r61096 + r61098 + r61099 in r62430, Closing.

for the record: completed in r62473.

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