Opened 9 years ago

Closed 9 years ago

#3870 closed defect (fixed)

osgeo.ogr.Feature.ExportToJson() error

Reported by: doug Owned by: hobu
Priority: normal Milestone: 1.8.0
Component: PythonBindings Version: unspecified
Severity: normal Keywords: json
Cc: hobu, Even Rouault

Description

Getting this error: ../osgeo/ogr.py", line 2398, in ExportToJson?

'geometry': self.GetGeometryRef?().ExportToJson?(as_object=True),

TypeError?: ExportToJson?() got an unexpected keyword argument 'as_object'

Patch attached.

Attachments (2)

osgeo.ogr.Feature.ExportToJson.patch (1.6 KB) - added by doug 9 years ago.
Feature_ExportToJson.patch (1.6 KB) - added by Even Rouault 9 years ago.
Fixed version of the initial proposed patch

Download all attachments as: .zip

Change History (6)

Changed 9 years ago by doug

comment:1 Changed 9 years ago by warmerdam

Cc: hobu Even Rouault added
Component: defaultPythonBindings
Keywords: json added
Owner: changed from warmerdam to hobu

Even / Howard,

Does this make sense? Do we depend on an external json package that has changed names?

comment:2 Changed 9 years ago by Even Rouault

The Feature.ExportToJson?() was added as part as #1891, so Howard / Chris Schmidt are the reference here ;-)

My understanding of the situation....

0) The patch is not appropriate in its current form since it patches a generated file. It should be a patch in swig/include/python/ogr_python.i instead

1) The issue is that Feature.ExportToJson?() seems to be broken and I doubt it has ever worked properly. It relies on a non existing mode of Geometry.ExportToJson?() that would export the geometry as a Python object hierarchy instead of a string (the call to self.GetGeometryRef?().ExportToJson?(as_object=True)). Hence the hack in the proposed patch to add a kwargs argument to Geometry.ExportToJson?(), but that's not the correct solution. A possible fix would be deserialize the geometry json string with json.loads()/simplejson.loads().

2) The json / simplejson thing is a bit clearer. According to http://docs.python.org/library/json.html and http://pypi.python.org/pypi/simplejson/ , simplejson was the only existing solution (as an external optional module) until python 2.6 appeared. Since python 2.6, there's a builtin json module available. So that part of the patch makes sense. Python >= 2.6 has always json, but not necessarily simplejson. However the usefulness of that part is a bit mitigated by point 1)

I suggest the attached patch instead, which also fixes an exception when the feature has no geometry.

Changed 9 years ago by Even Rouault

Attachment: Feature_ExportToJson.patch added

Fixed version of the initial proposed patch

comment:3 in reply to:  2 Changed 9 years ago by doug

Replying to rouault:

The patch is not appropriate in its current form since it patches a generated file.

Agreed.

I'm new to GDAL and not familiar with how the python lib is built, but when I saw this code I knew it needed a patch. It was only afterward I realized I submitted a patch for generated code.

The activity on this ticket is great and it inspires strong confidence in the code. Thanks all.

comment:4 Changed 9 years ago by Even Rouault

Milestone: 1.8.0
Resolution: fixed
Status: newclosed

Patch commited in r21216. Test added in r21217

Note: See TracTickets for help on using tickets.