Ticket #163 (defect)

I'm not certain what the trouble is, but running this script

from shapely.geometry import asShape
p = asShape({'type': 'Point', 'coordinates': (0,0)})
assert p.equals(p)

under gdb shows that the underlying geometry does get corrupted

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1210095424 (LWP 23403)]
0xb7cb6c56 in GEOSEquals (g1=0x81c45a0, g2=0x81cf370) at geos_c.cpp:399
399                     result = g1->equals(g2);
(gdb) p g1
$5 = (class geos::geom::Geometry *) 0x81c45a0
(gdb) p g2
$6 = (class geos::geom::Geometry *) 0x81cf370
(gdb) p g2->isValid()
$7 = true
(gdb) p g2->getGeometryType()
$8 = {static npos = 4294967295, 
  _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x81cdb84 "Point"}}
(gdb) p g1->isValid()
Cannot access memory at address 0x34

First, I believe we can take a safe shortcut when evaluating the topological equality of 2 shapely geoms. If the objects are equal in the Python sense, then they are going to be topologically equal. Right?

Second, I think this means that we should no longer treat topological equality and Python equality as the same thing. I propose to eliminate __eq__ and __ne__ from shapely's BaseGeometry?.

See r1082.

Thanks for the patch Sean. Although I think it's a good thing to have this patch applied, I'm a little concerned with the fact that it hides the actual bug. I'm sure you'd agree with that.

I really don't understand why the segfault occurs when comparing a PointAdapter object to itself. From GEOS' perspective, it shouldn't make any difference whether the comparison is on the same Python object or on two different Python objects.

Anyway, I'll test your patch on monday; I'm pretty confident it'll work. Thx.

I found the trouble. See shapely/geometry/point.py line 229. To keep a point adapter's GEOS geom synchronized with the coordinate storage (GeoJSON object or whatever), I've implemented a little dance with geos, the pointer to a GEOS geometry. It keeps synch, but there's a bad side effect for any binary operation on the same object: one pointer is destroyed.

Instead, PointAdapter?.geom could store a hash of the underlying coordinate storage (the context) and only recalculate if that has changed. When the context is a coordinate sequence, we can simply hash() the sequence. When it is an object that implements the Numpy array protocol, we could hash the representation of the array_interface attribute.

It could still be a good idea to keep the shortcuts.

r1088 has a caching geometry proxy that seems to work fine in the point case.

Sean, shouldn't your change be done for the other geometry adapters? E.g. <http://trac.gispython.org/projects/PCL/browser/Shapely/trunk/shapely/geometry/polygon.py#L204>

Shapely trunk now works great for my use case.

We're now using the caching proxy for all adapter types. I removed the shortcuts since it seems that GEOS already handles these cases efficiently enough.