[GRASS-dev] [grass-code I][381] r.colors rules: some scripts broken

code I item #381, was opened at 2007-04-25 19:24
Status: Open
Priority: 3
Submitted By: Maciej Sieczka (msieczka)
Assigned to: Nobody (None)
Summary: r.colors rules: some scripts broken
Issue type: module bug
Issue status: None
GRASS version: CVS HEAD
GRASS component: raster
Operating system: all
Operating system version:
GRASS CVS checkout date, if applies (YYMMDD): 070425

Initial Comment:
Eg. in GRASS 6.3 scripts dir:

i.in.spotvgt: 213: r.colors ${NAME} rules=ndvi 2>&1 >/dev/null
i.in.spotvgt: 275: r.colors ${NAME}_filt rules=ndvi 2>&1 >/dev/null
r.in.srtm: 237: r.colors rul=srtm map=$TILEOUT

Such syntax leads now to "ERROR: Unable to load rules file srtm".

How do we fix it? Add the $GISBASE/etc/colors/ before the rule name? Looks like a broken backwards compatibility...

Could C modules be affected?

Maciek

----------------------------------------------------------------------

You can respond by visiting:
http://wald.intevation.org/tracker/?func=detail&atid=204&aid=381&group_id=21

http://wald.intevation.org/tracker/?func=detail&atid=204&aid=381&group_id=21
..

code I item #381, was opened at 2007-04-25 19:24
Summary: r.colors rules: some scripts broken

..

Eg. in GRASS 6.3 scripts dir:

i.in.spotvgt: 213: r.colors ${NAME} rules=ndvi 2>&1 >/dev/null
i.in.spotvgt: 275: r.colors ${NAME}_filt rules=ndvi 2>&1 >/dev/null
r.in.srtm: 237: r.colors rul=srtm map=$TILEOUT

Such syntax leads now to "ERROR: Unable to load rules file srtm".

How do we fix it? Add the $GISBASE/etc/colors/ before the rule name?
Looks like a broken backwards compatibility...

[cc'd from the tracker]

r.colors was changed in 6.3 CVS,

-------
Revision 2.14, Sat Apr 14 03:35:32 2007 UTC by glynn
..
r.colors:
..
Merge rules= with color=
Use rules= for external rules file
-------

you can get rid of the errors by changing "r.colors rules="
to "r.colors color=". I've just done this for r.in.srtm.

But really this will break a lot of scripts and should be revisited
to be backwards compatible. Suggestion: when it looks for the rules
file it should try looking for $GISBASE/etc/colors/$RULEFILE if it
can't find the file as just $RULEFILE.

Anything written using "r.colors rules=" will be affected.

Hamish

Hamish wrote:

http://wald.intevation.org/tracker/?func=detail&atid=204&aid=381&group_id=21
..
> code I item #381, was opened at 2007-04-25 19:24
> Summary: r.colors rules: some scripts broken
..
> Eg. in GRASS 6.3 scripts dir:
>
> i.in.spotvgt: 213: r.colors ${NAME} rules=ndvi 2>&1 >/dev/null
> i.in.spotvgt: 275: r.colors ${NAME}_filt rules=ndvi 2>&1 >/dev/null
> r.in.srtm: 237: r.colors rul=srtm map=$TILEOUT
>
> Such syntax leads now to "ERROR: Unable to load rules file srtm".
>
> How do we fix it? Add the $GISBASE/etc/colors/ before the rule name?
> Looks like a broken backwards compatibility...

[cc'd from the tracker]

r.colors was changed in 6.3 CVS,

-------
Revision 2.14, Sat Apr 14 03:35:32 2007 UTC by glynn
..
r.colors:
..
Merge rules= with color=
Use rules= for external rules file
-------

you can get rid of the errors by changing "r.colors rules="
to "r.colors color=". I've just done this for r.in.srtm.

But really this will break a lot of scripts and should be revisited
to be backwards compatible. Suggestion: when it looks for the rules
file it should try looking for $GISBASE/etc/colors/$RULEFILE if it
can't find the file as just $RULEFILE.

Anything written using "r.colors rules=" will be affected.

When I added rules=, it was made clear that this was a temporary
feature until it could replace color=.

As there hasn't been a 6.3.0 release yet, the 6.3 API remains subject
to change.

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

Glynn Clements wrote:

> you can get rid of the errors by changing "r.colors rules="
> to "r.colors color=". I've just done this for r.in.srtm.
>
> But really this will break a lot of scripts and should be revisited
> to be backwards compatible. Suggestion: when it looks for the rules
> file it should try looking for $GISBASE/etc/colors/$RULEFILE if it
> can't find the file as just $RULEFILE.
>
> Anything written using "r.colors rules=" will be affected.

When I added rules=, it was made clear that this was a temporary
feature until it could replace color=.

I am happy for r.colors to evolve, and glad the rules files are now
finally merged into color=, but fear we will damage tons of user scripts
with this change.

"r.colors rules=" has been there for more than 3 years now, since before
the 6.0.0 release (5.3.0 and 5.7.0 too), and until recently the only way
to get at e.g. the srtm color rules, so widely used. All new rules since
then were added to rules= as it is so much easier than writing a new C
function for the old color= option.

As there hasn't been a 6.3.0 release yet, the 6.3 API remains subject
to change.

It is my understanding that module options are frozen for 6.x, not 6.x.y.
The idea being that a user script written for GRASS 6.0.0 will still
work with GRASS 6.9.99. Often an application is still in use at an
institution long after the original programmer/brain cells have moved on.

Would quietly checking for the file in $GISBASE/etc/colors/, after it
isn't found normally, really hurt much? (leaving that under-documented
in the help pages to discourage new use) We could flag it for removal
in GRASS7 if code clutter is a concern.

Hamish

Hamish wrote:

I am happy for r.colors to evolve, and glad the rules files are now
finally merged into color=, but fear we will damage tons of user scripts
with this change.

The longer it's left, the more scripts will be broken by the change.
It's already been left far too long (if it was originally done in 5.3,
it should have been changed for 6.0.0).

> As there hasn't been a 6.3.0 release yet, the 6.3 API remains subject
> to change.

It is my understanding that module options are frozen for 6.x, not 6.x.y.

That would make the rate of evolution far too slow. It would
essentially mean periods of several years where no real development
can occur.

If that was the case, I would have left after 6.0.0 was released, and
probably have forgotten all about GRASS by the time that 7.x was open
for development.

AIUI, major numbers are for "total" change: 5.x introduced FP and
null. 6.x (originally 5.7) completely changed the source directory
structure (no src/src.contrib/src.nonGPL, no cmd/inter
subdirectories), the build system, and the vector subsystem.

Minor numbers are for incompatible changes, release numbers are for
bug fixes and backward-compatible enhancements.

IOW, analogous to the Linux kernel: 2.0/2.2/2.4/2.6 all have
substantial differences, while all 2.6.x versions are mostly
compatible with each other.

Would quietly checking for the file in $GISBASE/etc/colors/, after it
isn't found normally, really hurt much?

Yes.

Either the argument to rules= is a filename or it isn't. If it is a
filename, relative filenames are relative to the current directory. If
it isn't a filename, then it's an arbitrary string, the GUI can't
offer a file browser for it.

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

FWIW, the old way rules= was implemented never made much sense. It should
have followed the old concept of rulesfile > r.colors and made it easier to
implement, but was a strange hybrid of this and colors=. The current
implementation is much more logical. I'm sympathetic about the problem with
user scripts, but it's easy to fix (change rules= to colors=).

Michael

On 5/3/07 12:36 AM, "Glynn Clements" <glynn@gclements.plus.com> wrote:

Hamish wrote:

Wald: Exiting with error
..

code I item #381, was opened at 2007-04-25 19:24
Summary: r.colors rules: some scripts broken

..

Eg. in GRASS 6.3 scripts dir:

i.in.spotvgt: 213: r.colors ${NAME} rules=ndvi 2>&1 >/dev/null
i.in.spotvgt: 275: r.colors ${NAME}_filt rules=ndvi 2>&1 >/dev/null
r.in.srtm: 237: r.colors rul=srtm map=$TILEOUT

Such syntax leads now to "ERROR: Unable to load rules file srtm".

How do we fix it? Add the $GISBASE/etc/colors/ before the rule name?
Looks like a broken backwards compatibility...

[cc'd from the tracker]

r.colors was changed in 6.3 CVS,

-------
Revision 2.14, Sat Apr 14 03:35:32 2007 UTC by glynn
..
r.colors:
..
Merge rules= with color=
Use rules= for external rules file
-------

you can get rid of the errors by changing "r.colors rules="
to "r.colors color=". I've just done this for r.in.srtm.

But really this will break a lot of scripts and should be revisited
to be backwards compatible. Suggestion: when it looks for the rules
file it should try looking for $GISBASE/etc/colors/$RULEFILE if it
can't find the file as just $RULEFILE.

Anything written using "r.colors rules=" will be affected.

When I added rules=, it was made clear that this was a temporary
feature until it could replace color=.

As there hasn't been a 6.3.0 release yet, the 6.3 API remains subject
to change.

__________________________________________
Michael Barton, Professor of Anthropology
School of Human Evolution & Social Change
Center for Social Dynamics & Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

[you should probably read this bottom-up, as solutions>>rhetoric.
hopefully the attached script demonstrates that there's really not much
of an issue here after all -> our ideas are not mutually exclusive]

Hamish wrote:
> I am happy for r.colors to evolve, and glad the rules files are now
> finally merged into color=, but fear we will damage tons of user
> scripts with this change.

Glynn:

The longer it's left, the more scripts will be broken by the change.

As long as the functionality is no longer advertised, no new scripts
should use it (or at least fewer and fewer).

It's already been left far too long (if it was originally done in 5.3,
it should have been changed for 6.0.0).

ok, but water under the bridge now.

GC:

> > As there hasn't been a 6.3.0 release yet, the 6.3 API remains
> > subject to change.

HB:

> It is my understanding that module options are frozen for 6.x, not
> 6.x.y.

GC:

That would make the rate of evolution far too slow. It would
essentially mean periods of several years where no real development
can occur.

If that was the case, I would have left after 6.0.0 was released, and
probably have forgotten all about GRASS by the time that 7.x was open
for development.

ok, "frozen" was perhaps a bad choice of word. my meaning was "existing
command line options locked in for the duration". Repeating my main
concern: a user script written for GRASS 6.0.0 should still work with
GRASS 6.9.99. People can adapt to GUI changes easy enough, scripts are
not so flexible.

AIUI, major numbers are for "total" change: 5.x introduced FP and
null. 6.x (originally 5.7) completely changed the source directory
structure (no src/src.contrib/src.nonGPL, no cmd/inter
subdirectories), the build system, and the vector subsystem.

yes; as far as the user is concerned the data files may not be
compatible between major versions.

Minor numbers are for incompatible changes,

That was not true for 6.0->6.2, we held that to mostly incremental
improvements. In fact I don't know of a single incompatible command line
change we did*. It would be a shame to first break our compatibility
guarantee so close to 7.0, as a little discipline on our part translates
into a lot of goodwill.

[*] outside of 1 option change in v.surf.rst? which was fixing an error

release numbers are for bug fixes and backward-compatible
enhancements.

right.

IOW, analogous to the Linux kernel: 2.0/2.2/2.4/2.6 all have
substantial differences, while all 2.6.x versions are mostly
compatible with each other.

Linux kernel devel is not 100% relevant, but FwIW my understanding is
that the 2.6 line has diverged into a continual upgrade cycle (no plans
for an unstable 2.7 or next-gen 2.8).

> Would quietly checking for the file in $GISBASE/etc/colors/, after
> it isn't found normally, really hurt much?

Yes.

Either the argument to rules= is a filename or it isn't.

it *is* a file name. I'm just asking to quietly augment the search path.

If it is a filename, relative filenames are relative to the current
directory.

$GISBASE should start with a root "/" or "C:", so no problem there.

Even if Tcl does not allow full path names in the file browser, that's
not a problem as 1) I'm talking about passing options from a script, and
2) if the user was hell-bent on using rules= in the old way from the
auto-gen GUI window, they can type whatever string they want in the
options box - it's not the file browser until you click on the file
folder icon.

If it isn't a filename, then it's an arbitrary string, the GUI can't
offer a file browser for it.

gisprompt->old_file is a good thing- of course the "new" rules= file
option should use it. The remaining question is if the parser checks for
the existence of the file when using old_file, or if it is up to the
module to do that. If it's up to the module then we can code in a path-
search trick to keep backwards compatibility, aka no problem. And this
seems to be the case -- try the attached script from the command line
using "srtm" as the file name. It works, and is all I am asking for the
C code to do.

anyway, yet another reason to push the move to SVN and begin the 7.0
branch so these discussions become moot.

Hamish

ps- all scripts/ in 6.3 CVS are now updated to use "r.colors color="
instead of rules=.

(attachments)

g.testfile.sh (738 Bytes)