GEOSIndexError when comparing geometries

[Craig de Stigter]

In 1.5 (latest master) I get the following when comparing geometries of different lengths:

from django.contrib.gis.geos import *
a = Polygon(((0, 0), (0, 1), (1, 1), (1, 0), (0, 0)))
b = Polygon(((0, 0), (0, 1), (1, 0), (0, 0)))

a < b

Traceback (most recent call last):
  File "<string>", line 6, in <module>
  File "/home/cdestigter/checkout/django/django/contrib/gis/geos/mutable_list.py", line 164, in __lt__
    c = self[i] < other[i]
  File "/home/cdestigter/checkout/django/django/contrib/gis/geos/mutable_list.py", line 164, in __lt__
    c = self[i] < other[i]
  File "/home/cdestigter/checkout/django/django/contrib/gis/geos/mutable_list.py", line 80, in __getitem__
    index = self._checkindex(index)
  File "/home/cdestigter/checkout/django/django/contrib/gis/geos/mutable_list.py", line 245, in _checkindex
    raise self._IndexError('invalid index: %s' % str(index))
django.contrib.gis.geos.error.GEOSIndexError: 'invalid index: 4'

This does not occur on django 1.4.x.

git bisect tracks it down to this commit: ​https://github.com/django/django/commit/d04f72fb31bab43386e12963c1c6dec23ae16143 ("Got rid of old cmpmethods replaced by rich comparison.")

python 2.6.5, GEOS 3.3.3-CAPI-1.7.4

marking as release blocker since it's a regression in 1.5.

[Craig de Stigter]

simple fix: ​https://github.com/koordinates/django/commit/751ebb7132de7d50a47114677892a3848de271b6

[Claude Paroz]

Craig, can you check the patch?
I kept catching IndexError, because it is possible that other is a simple list or any other object which may raise a plain IndexError.
I have also identified and fixed another regression in the equality method.

[Craig de Stigter]

My patch iterates over range(len(other)) to avoid getting an other._IndexError (which may not be either IndexError or self._IndexError). I dropped the catching of IndexError as it seems unnecessary in this case.

I also found another bug in __lt__ causing it to report incorrect results sometimes. I've added a test case to my patch to show the issue but haven't fixed the bug as I'm not quite sure the most efficient way to fix it.

[Craig de Stigter]

19366-2.diff plus fix lt when geometries are different lengths

[Claude Paroz]

After some thoughts, I think that comparing the linestrings for the equality makes sense, but not for > or <, because this will depend on the first differing point on the line string, which has absolutely nothing to do with being greater or smaller.
Shouldn't we override the __lt__ operator for polygons and compare the area instead?

[Craig de Stigter]

Not necessarily. Calculating a polygon's area is pretty expensive, and if all you want is a *consistent* ordering for a list of geometries (eg for hashing them consistently), simply ordering based on the coordinates is probably a lot faster.
At the very least, changing it implies a backwards compatibility break for anyone using the old ordering.

[Claude Paroz]

So I've added a documentation paragraph about Polygon comparison.
To solve the __lt__ issue, I don't think we can prevent using a second reverse comparison. Also done in that patch.
I also removed the import * you added in previous patches, was this line intended?

[Craig de Stigter]

Cool, that looks pretty good now. No, the import * wasn't meant to be in the patch. I had trouble figuring out how to run the test_geos.py tests, so that was my hackfix. Presumably there's a legit way to do it. They don't seem to run as part of the contrib.gis tests :/

[Claude Paroz]

Thanks for the help and review.
Tests are run with gis tests, however it is not possible to run individual tests or TestCases because of #6712.

[Claude Paroz <claude@…>]

In cc3c4a9d55e765cfe664e3dbd58201de99232096:

Fixed #19366 -- Prevented GEOSIndexError when comparing geometries

Thanks Craig de Stigter for the report and collaboration on the
patch.

[Claude Paroz <claude@…>]

In 89593048e2eb5ef64a51da43728307e2dbef5783:

[1.5.x] Fixed #19366 -- Prevented GEOSIndexError when comparing geometries

Thanks Craig de Stigter for the report and collaboration on the
patch.
Backport of cc3c4a9d5 from master.