[Community] Geometry subclassing and factory methods

[Sean Gillies]

On Apr 24, 2006, at 9:35 AM, Howard Butler wrote:

> All,
>
> I have run into an issue with the Geometry.fromWKT and .fromWKB  
> methods not supporting subclassing in the way I would like to use  
> it.  Sean and Kai said I should bring the issue up with the list  
> after some discussion on IRC and in the tracker, so here it is.
>
> The problem
> ------------------
>
> I would like to create a subclass of Point that does a weird buffer  
> (uses a hexagon instead of GEOS' buffer).
>
>> class Structure(Point):
>>     def buffer(self, side):
>>         radical_3 = math.sqrt(3.0)
>>         shell = LinearRing([
>>                                  Point(self.x - side*2.0/ 
>> radical_3, self.y),
>>                                  Point(self.x - side/radical_3,  
>> self.y + side),
>>                                  Point(self.x + side/radical_3,  
>> self.y + side),
>>                                  Point(self.x + side*2.0/ 
>> radical_3, self.y),
>>                                  Point(self.x + side/radical_3,  
>> self.y - side),
>>                                  Point(self.x - side/radical_3,  
>> self.y - side),
>>                                  Point(self.x - side*2.0/ 
>> radical_3, self.y)
>>                                  ])
>>         return Polygon(shell)
>
> Things behave as expected when instantiating using the normal  
> __init__ method of Structure:
>
>> init_struct = Structure(3.0, 4.0)
>> print "Type for Structure using __init__ : ",type(init_struct)
>>> Type for Structure using __init__ :  <class '__main__.Structure'>
>
> The problem comes when using the fromWKT (or fromWKB) factory  
> method to instantiate a Structure:
>
>> factory_struct = Structure.fromWKT(init_struct.toWKT())
>> print "Type for factory Structure : ",type(factory_struct)
>>> Type for factory Structure :  <class  
>>> 'cartography.spatial.geometry.Point'>
>
> factory_struct comes back as a Point, not a Structure, so calling  
> factory_struct.buffer() will be using the normal buffer function  
> instead of the weird hexagon one.
>
> How we can fix this (if it really needs fixing)
> ---------------------------------------------------------------
>
> The fromWKT and fromWKB factory (classmethod) functions in Geometry  
> have some code in them to set their __class__ using the  
> GEOSGeometry TypeId.  The GEOSGeometry is updated with the WKT, the  
> type (Polygon, Point, LineString, etc) is grabbed from the  
> GEOSGeometry, and then the instance's __class__ is set using a  
> lookup table.  All of the geometry types that inherit from Geometry  
> in geometry.py are supported this way.  The __class__ currently can  
> only be set to those ids that are in the lookup table -- not to the  
> actual class that called the fromWK[TB] method originally (in my  
> case the Structure class)
>
> This can be fixed by checking if we have a super and setting the  
> __class__ to that if it exists, otherwise use the normal lookup.
>> if super(cls):
>>     g.__class__= super(cls).__thisclass__
>> else:
>>     g.__class__ = CLASS_TYPE[g.getTypeId()]
>
>
> Arguments for doing this
> -----------------------------------
>
> Sean has provided some push back against making this change <http:// 
> trac.gispython.org/projects/PCL/ticket/68>, stating that it was a  
> rare use case, and it complicates the factory methods.  Here's my  
> arguments for making this change:
>
> 1) The geometry factory methods and their normal constructors are  
> currently incongruent with respect to subclassing (see the above  
> example).
> 2) Geometry already does a specialized version of this in its  
> factory methods, upcasting the Geometry to Point, Line, Polygon,  
> etc based on the on the underlying GEOSGeometry TypeId.  This takes  
> it all the way, casting the __class__ to the class that originally  
> called the factory.
>
> Thoughts, discussion, boos and cheers are welcomed.
>
> Howard
>

The original intent of the Geometry.fromWKT and Geometry.fromWKB  
factory methods is to take WKT or WKB encoded geometries (see OGC  
05-126, aka "Simple features access" for details) and, without *any*  
a priori knowledge of the geometry type encoded in the WKT/WKB  
string, instantiate from these strings geometry objects of the  
*exact* same type that is encoded. The only geometry types that can  
be encoded in WKT/WKB are:

- Point
- LineString
- Polygon
- MultiPoint
- MultiLineString
- MultiPolygon
- GeometryCollection

It was my intent that Python geometries of these types *only* would  
be created by these particular factory methods. This is a common  
capability of GIS packages. Where PCL differs from OGR or MapServer  
is that we exploit the mutability of Python objects to return  
instances of Point, LineString, Polygon, MultiPoint, etc classes from  
the factories.

I never intended that the factories should be used via sub-classes of  
Geometry, because this could lead to confusing behavior. For example,

   g = Polygon.fromWKT("POINT (0 0)")   # example 1

Although unfortunately allowed in the current state of PCL, this is  
nonsensical. In hindsight, I should have written  
createGeometryFromWK* functions. Perhaps it's not too late to drop  
Geometry.fromWK*.

Now, to the specific issue of sub-classing Point like, and modifying  
Geometry.fromWK* so that you can create instances of this new class.  
To me it seems much better if your new class accepts *all* the  
responsibility for setting the appropriate class. Why not

   class Structure(geometry.Point):
       ...
       def fromWKT(cls, wkt):
           g = geometry.Geometry()
           g._updateGeomFromWKT(wkt)
           g.__class__ = cls
           return g
       fromWKT = classmethod(fromWKT)

? This keeps a rarely needed if/else out of Geometry.fromWK*, which I  
strongly prefer. And now that your new class is taking full  
responsibility, you can implement a guard against your users passing  
the wrong geometry types into the factory method. This is where  
example 1 reappears in a different form:

   g = Structure.fromWKT("POLYGON ...")

This has to raise an exception, right? In order to have a Structure  
class that does the right thing when given bogus data, you'll need to  
do something like this:

   class Structure(geometry.Point):
       ...
       def fromWKT(cls, wkt):
           g = geometry.Geometry()
           g._updateGeomFromWKT(wkt)
           if g.getTypeId() is not geometry.GEOS_POINT:
	      raise Exception, "Invalid WKT, only POINT is permissable"
           g.__class__ = cls
           return g
       fromWKT = classmethod(fromWKT)

I don't think we want to push that geometry type checking into  
Geometry.fromWKT because it would require a rarely necessary hash  
lookup.

That's my suggestion, and mostly based on my sense that the need for  
highly customized geometry types is rare. Rare enough that users of  
the highly customized geometries should accept all responsibility for  
decoding WKT/WKB into non-standard types. Overwriting fromWK* for  
this specialized Structure class just doesn't seem to be that much of  
a burden.

So why do you want hexagonally buffered points anyway? Why not use  
points and hexagons?

Sean

---
Sean Gillies
http://zcologia.com