#2343: GRASS 7: "inf" values break insert statements in v.rast.stats
--------------------------+-------------------------------------------------
Reporter: sbl | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-releasebranch70
Keywords: v.rast.stats | Platform: All
Cpu: All |
--------------------------+-------------------------------------------------
When running v.rast.stats on a dataset with small geometries (which in my
case were represented by 2 pixels in the raster), the parameter
"coeff_var" can result in "inf" value.
"inf" breaks the INSERT statement and results in an DBMI error "no such
column: inf", which at the end crashes v.rast.stats.
Just like "nan", "inf" could be replaced by NULL. Changing line 263 in
v.rast.stats.py into this:
if value.lower().endswith('nan') or value.lower().endswith('inf'):
should work.
Both GRASS 7.0 and 7.1 are affected. Please find attached patches for both
versions...
#2343: GRASS 7: "inf" values break insert statements in v.rast.stats
--------------------------+-------------------------------------------------
Reporter: sbl | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-releasebranch70
Keywords: v.rast.stats | Platform: All
Cpu: All |
--------------------------+-------------------------------------------------
Comment(by glynn):
Replying to [ticket:2343 sbl]:
> "inf" breaks the INSERT statement and results in an DBMI error "no such
column: inf", which at the end crashes v.rast.stats.
>
> Just like "nan", "inf" could be replaced by NULL.
FWIW, PostgreSQL supports infinity and NaN values, e.g.
{{{
CREATE TABLE test ( value REAL ) ;
INSERT INTO test VALUES (REAL 'Infinity');
INSERT INTO test VALUES (REAL '+Infinity');
INSERT INTO test VALUES (REAL '-Infinity');
INSERT INTO test VALUES (REAL 'NaN');
SELECT * FROM test ;
value
-----------
Infinity
Infinity
-Infinity
NaN
(4 rows)
}}}
I don't know whether this is part of the SQL standard, or whether it is
supported by the other back-ends. It won't work for columns of type
NUMERIC.
#2343: GRASS 7: "inf" values break insert statements in v.rast.stats
--------------------------+-------------------------------------------------
Reporter: sbl | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-releasebranch70
Keywords: v.rast.stats | Platform: All
Cpu: All |
--------------------------+-------------------------------------------------
Comment(by sbl):
Replying to [comment:2 glynn]:
> FWIW, PostgreSQL supports infinity and NaN values, e.g.
> I don't know whether this is part of the SQL standard, or whether it is
supported by the other back-ends. It won't work for columns of type
NUMERIC.
Thanks for the hint, I was not aware of that.
Now I investigated a bit and it seems that SQLite (as default DBMI in
GRASS 7) also supports NaN and Infinity. However, using quoted literal
strings instead of NULL in v.rast.stas (regardless if it is "NaN", "inf",
"Infinity", "INF" ...), always resulted in a value 0 in the attribute
table (in my example at least). So, supporting NaN or Inf is unfortunately
not as straightforward as I hoped...
#2343: GRASS 7: "inf" values break insert statements in v.rast.stats
--------------------------+-------------------------------------------------
Reporter: sbl | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-releasebranch70
Keywords: v.rast.stats | Platform: All
Cpu: All |
--------------------------+-------------------------------------------------
Comment(by lucadelu):
Replying to [ticket:2343 sbl]:
> When running v.rast.stats on a dataset with small geometries (which in
my case were represented by 2 pixels in the raster), the parameter
"coeff_var" can result in "inf" value.
> "inf" breaks the INSERT statement and results in an DBMI error "no such
column: inf", which at the end crashes v.rast.stats.
>
> Just like "nan", "inf" could be replaced by NULL. Changing line 263 in
v.rast.stats.py into this:
> if value.lower().endswith('nan') or value.lower().endswith('inf'):
> should work.
>
> Both GRASS 7.0 and 7.1 are affected. Please find attached patches for
both versions...
#2343: GRASS 7: "inf" values break insert statements in v.rast.stats
--------------------------+-------------------------------------------------
Reporter: sbl | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-releasebranch70
Keywords: v.rast.stats | Platform: All
Cpu: All |
--------------------------+-------------------------------------------------
Comment(by lucadelu):
Replying to [comment:5 sbl]:
> Replying to [comment:4 lucadelu]:
> > Could you provide a dataset to test the patches?
>
> You can reproduce the issue with these commands:
>
> {{{
> g.region -p n=1 s=0 e=2 w=0 res=1
> r.mapcalc --o expression="rastermap1=1"
> r.mapcalc --o expression="rastermap2=double(if(x()==0.5,-1,1))"
> r.to.vect --o --v input=rastermap1 output=vectormap type=area
> v.rast.stats -c --verbose map=vectormap raster=rastermap2
column_prefix=value
>
> }}}
In r62938 I should fixed the error.
I change a little bit your patch to cover multiple way to catch Infinity
value.
If it is working I'll backport the changeset to GRASS70.
I also applied your patch for GRASS64 version in r62944.
#2343: GRASS 7: "inf" values break insert statements in v.rast.stats
--------------------------+-------------------------------------------------
Reporter: sbl | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-releasebranch70
Keywords: v.rast.stats | Platform: All
Cpu: All |
--------------------------+-------------------------------------------------
Comment(by glynn):
Replying to [comment:3 sbl]:
> At the end it is also a question wether NaN or Inf is preferable over
NULL in the database...
Well, for rasters, NaN is null (in 6.x, one specific NaN is treated as
null, in 7.x any NaN value is treated as null). Also, code which is
expecting a definite value probably won't handle a NaN correctly because
of their unusual semantics.
Ideally, infinities should be preserved rather than converted to an SQL
NULL. If this is just a matter of fixing the int<->string conversions
(rather than blindly using e.g. sprintf/sscanf "%f"), then it should be
done.
#2343: GRASS 7: "inf" values break insert statements in v.rast.stats
--------------------------+-------------------------------------------------
Reporter: sbl | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-releasebranch70
Keywords: v.rast.stats | Platform: All
Cpu: All |
--------------------------+-------------------------------------------------
Comment(by glynn):
Replying to [comment:8 neteler]:
> ...should r62938 be reverted and implemented differently?
Maybe. It depends upon how much work is involved.
Infinities are sufficiently rare that it probably doesn't matter how
they're handled (e.g. r.mapcalc explicitly checks for division by zero,
log(0) etc and returns null). If it's just a matter of writing
float_to_string() and string_to_float() functions which deal with them
correctly (and we know how to write them), it should be done. If there are
other complexities, or if every supported database has a different
representation, don't bother.
Storing NaNs in a database is probably undesirable. I wouldn't assume that
all databases will implement the same semantics, or even sane semantics
(the fact that they aren't equal to themselves, that all of x<NaN, x>NaN,
NaN<x and NaN>x are false, etc tends to wreak havoc with any algorithm
which isn't expecting them). Code which retrieves values from a database
is more likely to have been written to handle SQL NULLs correctly than to
handle NaNs correctly.