Opened 17 years ago
Closed 15 years ago
#1442 closed defect (fixed)
Feature.GetFieldAsInteger does not behave correctly on non existant field
Reported by: | Owned by: | Even Rouault | |
---|---|---|---|
Priority: | normal | Milestone: | 1.7.0 |
Component: | OGR_SF | Version: | 1.3.2 |
Severity: | normal | Keywords: | |
Cc: | warmerdam, hobu |
Description (last modified by )
Using Feature.GetFieldAsInteger on a inexistant field in the feature, answer is always 0. I guess it should report a ValueError like Feature.GetField (or even an AttributeError).
Demonstration :
did@geru-itae:~/Documents/ucl/alert/python$ python Python 2.4.4 (#2, Jan 13 2007, 17:50:26) [GCC 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> import ogr >>> dataset = ogr.Open('alert/test/scenario1/polygones.shp') >>> layer = dataset.GetLayer() >>> feature = layer.GetNextFeature() >>> print "Test with an existing field" Test with an existing field >>> feature.GetFieldIndex('cat') 0 >>> feature.GetFieldAsInteger(feature.GetFieldIndex('cat')) 1 >>> print "Test with a non existant field" Test with a non existant field >>> feature.GetFieldIndex('id') -1 >>> feature.GetFieldAsInteger(feature.GetFieldIndex('id')) 0 >>> feature.GetField(feature.GetFieldIndex('id')) Traceback (most recent call last): File "<stdin>", line 1, in ? File "/usr/lib/python2.4/site-packages/ogr.py", line 854, in GetField return _gdal.OGR_F_GetField( self._o, fld_index ) ValueError: Illegal field requested in GetField().
Change History (10)
comment:3 by , 17 years ago
Cc: | added |
---|---|
Description: | modified (diff) |
Milestone: | → 1.4.2 |
Owner: | changed from | to
Priority: | highest → normal |
comment:4 by , 17 years ago
Description: | modified (diff) |
---|
comment:5 by , 17 years ago
Status: | new → assigned |
---|
First, I confirm the problem still exists (see attached script, easier to test).
None of GetFieldAs* functions from the Feature class include testing if field index value is valid.
Currently, user has no chance to find out there is something wrong because everything behaves silently. I think users should be notified that requested field is not available in a feature attributes.
Summarizing, I think it's a good idea to throw ValueError if requested field does not exist.
comment:6 by , 17 years ago
Mateusz,
Did you have a plan how to address this? I'm inclined to modify the c++ methods on OGFeature to issue a CPLError() if the field is out of range. I believe that the swig bindings will automatically turn this into an exception depending on the rules for the language. In python I believe CPLErrors are not turned into exceptions by default, but things can be put into that mode if desired.
I do not think this changes should go into 1.4 branch.
comment:7 by , 17 years ago
Milestone: | 1.4.2 → 1.5.0 |
---|
comment:8 by , 17 years ago
Frank,
My first thought was related to the Traditional Python and here I've been thinking about manual hacking in gdal_wrap.c:
if( iField < 0 || iField >= OGR_F_GetFieldCount( hFeat ) ) { PyErr_SetString(PyExc_ValueError, "Illegal field requested in GetField()." ); return NULL; }
For the NG Python, the idea of automatic issue of Python exception on CPLError() sounds good to me. Also, I think it's a good idea to consult it with Hobu.
comment:9 by , 17 years ago
Cc: | added |
---|
Mateusz,
I do not think that we want to address this in the bindings code, and we certainly don't want to bother with updates to the old generation python bindings.
Howard, can you verify that the ng bindings can turn a CPLError into an exception if we issue a CPLError down in the C code?
comment:10 by , 16 years ago
Milestone: | 1.5.0 → 1.6.0 |
---|
I have confirmed that the ng bindings behave similarly:
>>> from osgeo import gdal >>> from osgeo import ogr >>> ds = ogr.Open('/Users/hobu/svn/gdalautotest/ogr/data/asm.shp') >>> layer = ds.GetLayer(0) >>> feature = layer.GetNextFeature() >>> feature <osgeo.ogr.Feature; proxy of <Swig Object of type 'OGRFeatureShadow *' at 0x34b2f0> > >>> feature.GetFieldIndex('GEO_ID') 5 >>> feature.GetFieldAsInteger(feature.GetFieldIndex('GEO_ID')) 16 >>> feature.GetFieldAsInteger(feature.GetFieldIndex('id')) 0 >>> ogr.UseExceptions() >>> feature.GetFieldAsInteger(feature.GetFieldIndex('id')) 0 >>> feature.GetField(feature.GetFieldIndex('junk')) Traceback (most recent call last): File "<stdin>", line 1, in <module> File "build/bdist.macosx-10.5-i386/egg/osgeo/ogr.py", line 1835, in GetField ValueError: Illegal field requested in GetField() >>> feature.GetFieldAsInteger(feature.GetFieldIndex('junk')) 0
I think this stuff should be handled at the C/C++ level so that bindings can decide what to do with the error. I'm not sure how to go about it though. I am deferring this bug until 1.6.
comment:11 by , 15 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
comment:12 by , 15 years ago
Milestone: | 1.6.1 → 1.7.0 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
r16845: Issue a CPLError if OGRFeatureDefn::GetFieldDefn() is called with an invalid index
At the beginning I had thought of adding the error in OGRFeatureDefn::GetFieldIndex( const char * pszFieldName ), but many OGR drivers use this method to detect if a field exists or not, so it was not appropriate to put it there.
Mateusz,
Chat with me about what you think the solution should be before implementing it for this.