[gdal-dev] 2.2.0beta1 regression NULL values

[Roger Bivand]

Hi Even,

On Fri, 21 Apr 2017, Even Rouault wrote:

> Hi Roger,
>
>> See https://github.com/edzer/sfr/issues/309 - < 2.2.0 we could write
>> "missing values" in vector drivers and read them back. In 2.2.0beta1, we
>> see REAL "missing values" written out OK, but read back as numeric zero.
>
> Did you check the output with ogrinfo/GDAL 2.2 to verify that they are indeed properly
> written as missing ?
>
>> We've only checked shapefiles and GPKG so far.
>>
>> Is this RFC 67?
>
> Most likely
>
>> Until now, rgdal code (and likely sf) has used:
>>
>>        case OFTReal:
>>  	if (poFeature->IsFieldSet(iField))
>>            REAL(ans)[iRow]=poFeature->GetFieldAsDouble(iField);
>>  	else REAL(ans)[iRow]=NA_REAL;
>>  	break;
>>
>> and the same for other field and list field types. Should we be using:
>>
>> int    CPL_DLL OGR_F_IsFieldSetAndNotNull( OGRFeatureH, int );
>
> Yes
>
>>
>> and should we be writing:
>>
>> void   CPL_DLL OGR_F_SetFieldNull( OGRFeatureH, int );
>>
>> instead of leaving the field unset with for example:
>>
>>               if (!ISNA(NUMERIC_POINTER(VECTOR_ELT(ldata, j))[i]))
>>                   poFeature->SetField( CHAR(STRING_ELT(fld_names, j)),
>>                       NUMERIC_POINTER(VECTOR_ELT(ldata, j))[i] );
>>
>> that is jumping over features for which the field value coming from R is
>> missing?
>
> Leaving the field unset or setting it to Null with OGR_F_SetFieldNull() 
> will have the same effect for most drivers. For example the shapefile 
> driver uses IsFieldSetAndNotNull() internally on the write side, so both 
> should result in the same result. The difference between unset and 
> setting to null will only be seen for JSon-based drivers or GML.

Have commited code changes for rgdal which now work the same for 
2.2.0beta1 and 2.1.3 both for files generated by GDAL version, and crossed 
(read 2.2.0beta1-generated files on 2.1.3 and vice-versa). Edzer made the 
changes a week ago in sf.

>
>>
>> We'd need to condition on GDAL version here (too), I guess.
>>
>> Was this regression intended?
>
> There are indeed a few backward incompatible changes (that you've 
> mentionned above) per
> https://trac.osgeo.org/gdal/wiki/rfc67_nullfieldvalues that require user 
> code changes.

Not a regression as far as I can now see, just a consequence of the 
"backward incompatible changes" that will affect many downstream users of 
many drivers, I guess.

Thanks for getting back so quickly,

Roger

>
> Besides those intended changes, it is not impossible of course there's a 
> real regression in GDAL itself, but ideally you should try to isolate it 
> from the Rgdal code, possibly through a standalone C code or a GDAL 
> Python script, so it can be more easily investigated (unfortunately, I 
> couldn't really make sense of the ticket you mentionned due to my 
> unfamiliarity with rgdal or sf). I'd also start by making sure where the 
> issue is really: on the read side, or the write side. For example by 
> producing a shapefile/GPKG with rgdal/GDAL 2.1.3 and reading it with 
> rgdal/GDAL 2.2, and the reverse.
>
> Best regards,
>
> Even
>
>

-- 
Roger Bivand
Department of Economics, Norwegian School of Economics,
Helleveien 30, N-5045 Bergen, Norway.
voice: +47 55 95 93 55; e-mail: Roger.Bivand at nhh.no
Editor-in-Chief of The R Journal, https://journal.r-project.org/index.html
http://orcid.org/0000-0003-2392-6140
https://scholar.google.no/citations?user=AWeghB0AAAAJ&hl=en