v.in.ascii (bug #5209) [was Re: [GRASS-dev] ps.map: scale wrong and no output]

martinl · November 21, 2006, 8:17am

Hi,

2006/11/21, Hamish <hamish_nospam@yahoo.com>:

Martin Landa wrote:
> > https://intevation.de/rt/webrt?serial_num=5209
>
> I have tried to fix this bug (the attached patch). Is it OK? -- then I
> will commit it to CVS...

re. "column type" text, it is probably not a good idea to mix
fprintf(stderr, and G_message(). I can see why you kept the first bit
with fprintf (no newline), but the module output will look really weird
if called with the --quiet flag as only G_message() part would disappear.

ou, sorry, ... you are absolutely right...

What seems to be needed is a G__message() or G_message_no_newline() or
somesuch fn that doesn't terminate with a newline.

I have tried to modify error.c, there is the new G__message () fn.

Best regards, Martin

This could be used for "Percent complete:"+G_percent() as well.
Then both msgs get switched on/off together with quiet/verbose.

The following looks good to me, but to preserve precision should "%lf"
be used instead of "%f", perhaps combined with G_trim_decimal()?

Index: vector/v.in.ascii/points.c

RCS file: /home/grass/grassrepository/grass6/vector/v.in.ascii/points.c,v
retrieving revision 1.21
diff -u -r1.21 points.c
--- vector/v.in.ascii/points.c 18 Oct 2006 05:09:22 -0000 1.21
+++ vector/v.in.ascii/points.c 19 Nov 2006 16:45:25 -0000
@@ -368,7 +368,15 @@
                if (strlen(tokens[i]) > 0) {
                    if (coltype[i] == DB_C_TYPE_INT ||
                        coltype[i] == DB_C_TYPE_DOUBLE) {
- sprintf(buf2, "%s", tokens[i]);
+ if (G_projection() == PROJECTION_LL &&
+ (i == xcol || i == ycol)) {
+ if (i == xcol)
+ sprintf(buf2, "%f", x);
+ else
+ sprintf(buf2, "%f", y);
+ }
+ else
+ sprintf(buf2, "%s", tokens[i]);
                    }
                    else {
                        db_set_string(&val, tokens[i]);

thanks,
Hamish

--
Martin Landa <landa.martin@gmail.com> * http://gama.fsv.cvut.cz/~landa *

(attachments)

v_in_ascii_LL-2.diff.gz (1.53 KB)

martinl · November 21, 2006, 10:24am

Hi,

sorry, the diff package does not contain gisdefs.h ... correction attached.

Best regards, Martin

2006/11/21, Martin Landa <landa.martin@gmail.com>:

Hi,

2006/11/21, Hamish <hamish_nospam@yahoo.com>:
> Martin Landa wrote:
> > > https://intevation.de/rt/webrt?serial_num=5209
> >
> > I have tried to fix this bug (the attached patch). Is it OK? -- then I
> > will commit it to CVS...
>
> re. "column type" text, it is probably not a good idea to mix
> fprintf(stderr, and G_message(). I can see why you kept the first bit
> with fprintf (no newline), but the module output will look really weird
> if called with the --quiet flag as only G_message() part would disappear.

ou, sorry, ... you are absolutely right...

> What seems to be needed is a G__message() or G_message_no_newline() or
> somesuch fn that doesn't terminate with a newline.

I have tried to modify error.c, there is the new G__message () fn.

Best regards, Martin

> This could be used for "Percent complete:"+G_percent() as well.
> Then both msgs get switched on/off together with quiet/verbose.
>
> The following looks good to me, but to preserve precision should "%lf"
> be used instead of "%f", perhaps combined with G_trim_decimal()?
>
> Index: vector/v.in.ascii/points.c
> ===================================================================
> RCS file: /home/grass/grassrepository/grass6/vector/v.in.ascii/points.c,v
> retrieving revision 1.21
> diff -u -r1.21 points.c
> --- vector/v.in.ascii/points.c 18 Oct 2006 05:09:22 -0000 1.21
> +++ vector/v.in.ascii/points.c 19 Nov 2006 16:45:25 -0000
> @@ -368,7 +368,15 @@
> if (strlen(tokens[i]) > 0) {
> if (coltype[i] == DB_C_TYPE_INT ||
> coltype[i] == DB_C_TYPE_DOUBLE) {
> - sprintf(buf2, "%s", tokens[i]);
> + if (G_projection() == PROJECTION_LL &&
> + (i == xcol || i == ycol)) {
> + if (i == xcol)
> + sprintf(buf2, "%f", x);
> + else
> + sprintf(buf2, "%f", y);
> + }
> + else
> + sprintf(buf2, "%s", tokens[i]);
> }
> else {
> db_set_string(&val, tokens[i]);
>
> thanks,
> Hamish
>

--
Martin Landa <landa.martin@gmail.com> * http://gama.fsv.cvut.cz/~landa *

--
Martin Landa <landa.martin@gmail.com> * http://gama.fsv.cvut.cz/~landa *

(attachments)

v_in_ascii_LL-3.diff.gz (1.68 KB)

Paul_Kelly · November 21, 2006, 10:59am

Hello Martin,
Sorry to take the lazy approach instead of changing it myself - but the way you have done it there is code repetition between G_message() and G__message(). Code repetition (clones) is always bad because it means more places to change things if you're fixing a bug or changing something.

You should really write it so G_message() is actually a really short function that calls G__message() to do most of the work. If possible!
If you look at some of the other examples of pairs of G_xxx() and G__xxx() functions in gislib you will see this is how they work.

Paul

On Tue, 21 Nov 2006, Martin Landa wrote:

Hi,

sorry, the diff package does not contain gisdefs.h ... correction attached.

Best regards, Martin

2006/11/21, Martin Landa <landa.martin@gmail.com>:

Hi,

2006/11/21, Hamish <hamish_nospam@yahoo.com>:
> Martin Landa wrote:
> > > https://intevation.de/rt/webrt?serial_num=5209
> >
> > I have tried to fix this bug (the attached patch). Is it OK? -- then I
> > will commit it to CVS...
>
> re. "column type" text, it is probably not a good idea to mix
> fprintf(stderr, and G_message(). I can see why you kept the first bit
> with fprintf (no newline), but the module output will look really weird
> if called with the --quiet flag as only G_message() part would disappear.

ou, sorry, ... you are absolutely right...

> What seems to be needed is a G__message() or G_message_no_newline() or
> somesuch fn that doesn't terminate with a newline.

I have tried to modify error.c, there is the new G__message () fn.

Best regards, Martin

> This could be used for "Percent complete:"+G_percent() as well.
> Then both msgs get switched on/off together with quiet/verbose.
>
> The following looks good to me, but to preserve precision should "%lf"
> be used instead of "%f", perhaps combined with G_trim_decimal()?
>
> Index: vector/v.in.ascii/points.c
> ===================================================================
> RCS file: /home/grass/grassrepository/grass6/vector/v.in.ascii/points.c,v
> retrieving revision 1.21
> diff -u -r1.21 points.c
> --- vector/v.in.ascii/points.c 18 Oct 2006 05:09:22 -0000 1.21
> +++ vector/v.in.ascii/points.c 19 Nov 2006 16:45:25 -0000
> @@ -368,7 +368,15 @@
> if (strlen(tokens[i]) > 0) {
> if (coltype[i] == DB_C_TYPE_INT ||
> coltype[i] == DB_C_TYPE_DOUBLE) {
> - sprintf(buf2, "%s", tokens[i]);
> + if (G_projection() == PROJECTION_LL &&
> + (i == xcol || i == ycol)) {
> + if (i == xcol)
> + sprintf(buf2, "%f", x);
> + else
> + sprintf(buf2, "%f", y);
> + }
> + else
> + sprintf(buf2, "%s", tokens[i]);
> }
> else {
> db_set_string(&val, tokens[i]);
>
> thanks,
> Hamish
>

--
Martin Landa <landa.martin@gmail.com> * http://gama.fsv.cvut.cz/~landa *

--
Martin Landa <landa.martin@gmail.com> * http://gama.fsv.cvut.cz/~landa *

H_B · November 22, 2006, 4:37am

https://intevation.de/rt/webrt?serial_num=5209
DDD:MM:SS -> xxx.xxxxxxx breaks during DB population stage

Martin:

> I have tried to fix this bug (the attached patch). Is it OK? --
> then I will commit it to CVS...

Hamish:

The following looks good to me, but to preserve precision should
"%lf" be used instead of "%f", perhaps combined with
G_trim_decimal()?

Index: vector/v.in.ascii/points.c

/home/grass/grassrepository/grass6/vector/v.in.ascii/points.c,v
retrieving revision 1.21 diff -u -r1.21 points.c
--- vector/v.in.ascii/points.c 18 Oct 2006 05:09:22 -0000
1.21 +++ vector/v.in.ascii/points.c 19 Nov 2006 16:45:25 -0000
@@ -368,7 +368,15 @@
                if (strlen(tokens[i]) > 0) {
                    if (coltype[i] == DB_C_TYPE_INT ||
                        coltype[i] == DB_C_TYPE_DOUBLE) {
- sprintf(buf2, "%s", tokens[i]);
+ if (G_projection() == PROJECTION_LL &&
+ (i == xcol || i == ycol)) {
+ if (i == xcol)
+ sprintf(buf2, "%f", x);
+ else
+ sprintf(buf2, "%f", y);
+ }
+ else
+ sprintf(buf2, "%s", tokens[i]);
                    }
                    else {
                        db_set_string(&val, tokens[i]);

In 6.3-cvs I have committed the above with "%.15g" replacing "%f".
DBF limits double columns to .6 precision, so I had to switch to SQLite
to see any difference. I think .9 is enough with lat/lon to get to
roughly 1mm precision, but cranked it up to .15 anyway.

However --- is it a better solution to change the column type to
varchar() rather than store the decimal equivalent in a double? ie
preserve the input data in its original form in the database.
(after checks that x,y,z post-conversion can all be stored as doubles
in the vector geometry)

[keeping bug open]

Hamish