Opened 13 years ago

Closed 13 years ago

#1255 closed defect (fixed)

ogr.Geometry __init__ method not functionnal with some attributes

Reported by: dpinte@… Owned by: hobu
Priority: highest Milestone: 1.4.1
Component: OGR_SF Version: unspecified
Severity: blocker Keywords:
Cc: dpinte@…

Description (last modified by hobu)

__init__ methods cannot return anything else than None in Python. If trying to return something else, it raises a TypeError.

When using ogr.Geometry constructor with wkt, wkb or gml parameters, the class cannot be used.

For example : 
org.py - line 1019 

       elif wkt:
            if srs:
                return CreateGeometryFromWkt(wkt, srs)
            else:
                return CreateGeometryFromWkt(wkt)

This causes the following error while trying to use ogr.Geometry(wkt=mywkt): 

======================================================================
ERROR: testpoly (__main__.TestIntrusionPoly)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "alert/test/testoutflow.py", line 125, in testpoly
    polylist = poly.create_polygons()
  File "/home/did/Documents/ucl/alert/python/alert/data/intrusion.py", line 460, in create_polygons
    coast_poly      = geometry.create_ogrgeom(ccorners, srs)
  File "/home/did/Documents/ucl/alert/python/alert/gis/geometry.py", line 71, in create_ogrgeom
    poly = ogr.Geometry(wkt=wktpoly)
TypeError: __init__() should return None

You should consider to rewrite this part of the code :
ogr.py - line 1019 :
        elif wkt:
            if srs:
                self._o = CreateGeometryFromWkt(wkt, srs)
                self.thisown = 1
            else:
                self._o = CreateGeometryFromWkt(wkt)
                self.thisown = 1


I'll make and send a patch in the next minutes.

Cheers,

Didrik

Attachments (1)

ogr_geometry.patch (1.4 KB) - added by dpinte@… 13 years ago.
Proposed patch

Download all attachments as: .zip

Change History (10)

Changed 13 years ago by dpinte@…

Attachment: ogr_geometry.patch added

Proposed patch

comment:1 Changed 13 years ago by dpinte@…

Some info about the patch :

Forget about my suggestions in my first post. I was wrong. 

The patch seems the right way to do it. Calling ogr.CreateGeometryFromWkt directly from the Geometry constructor gives a Segmentation fault. That's why I had to use the _gdal.OGR_G_CreateGeometryFromWkt function.


comment:2 Changed 13 years ago by hobu

Didrik,

I'm a little foggy on the description, but is the problem that the Create*from* methods are returning values?  Have you tried this with the next generation bindings?  Do they still cause the problem?

I'll take on this bug for now.  If you can recall any more details about it, I would greatly appreciate it.

Howard

comment:3 Changed 13 years ago by dpinte@…

Hi Howard,

Concerning the ng bindings, I haven't tested them but will in a near future.

About this specific bug problem, here are some clearer details : 

Python __init__ methods can not return a value. So if ogr wants to create a new geometry in the __init__ method, it can do it but without returning the created object. Python will automatically return a reference to the newly created Python object.

The patch I proposed is the best way to do it without changing the Python interface.

For example, the wkt case : 

-            if srs:
-                return CreateGeometryFromWkt(wkt, srs)
+            if srs is None:
+                srs = ''
             else:
-                return CreateGeometryFromWkt(wkt)
+                srs = srs._o
+            _obj  = _gdal.OGR_G_CreateFromWkt(wkt, srs)
+            if _obj is not None and _obj != 'NULL':
+                self._o = _obj
+                self.thisown = 1

The return CreateGeometryFromWkt(wkt, srs) line is not valid in Python. I've added some cleaner testing about the None value for srs, then created the _obj using _gdal.OGR_G_CreateFromWkt and finally updated the self._o object. This is all it has to do in the __init__ method.

Is it clear for you ?

Didrik

comment:4 Changed 13 years ago by dpinte@…

Hi Howard,

I've just tested the new generation bindings and everything works well. This is from the official version 1.4.0

BUT 

I guess that it's not possible to create a geometry with ogr.Geometry from wkt and with an srs (from ogr.py):

 __init__(self, OGRwkbGeometryType type=wkbUnknown, char wkt=0, int wkb=0, 

            char wkb_buf=0, char gml=0) -> Geometry



You can do it from ogr.CreateGeometryFromWkt but not anymore from ogr.Geometry.

Because it is possible to create a geometry from Wkt using ogr.Geometry(wkt=...), I guess that the API should allow the user to create a geometry with an srs like this ogr.Geometry(wkt=..., reference=mysrs).

I'm available if you need anymore information.

Didrik



comment:5 Changed 13 years ago by dpinte@…

To summarize the problem,

with version 1.3.x : Geometry.__init__ is broken when using srs, wkt, wkb or gml attribute. This can be easily corrected using my proposed patch

with version 1.4.x : the code runs fine.

Didrik

comment:6 Changed 13 years ago by warmerdam

Didrik, 

So does this mean there is no action since the problem is already corrected?
If so, please close this bug with status FIXED and an appropriate note. 

comment:7 Changed 13 years ago by dpinte@…

Frank,

I forgot one precision in my summary.

It works fine with version 1.4.x and the ng bindings. 

The 1.4.x with the old bindings has exactly the same problem has the one described for version 1.3.x and must be patched (pymod/ogr.py).

Didrik

comment:12 Changed 13 years ago by hobu

Description: modified (diff)
Milestone: 1.4.1
Owner: changed from warmerdam to hobu
Status: assignednew

comment:13 Changed 13 years ago by hobu

Resolution: fixed
Status: newclosed

committed in r11099.

Note: See TracTickets for help on using tickets.