From cc3c4a9d55e765cfe664e3dbd58201de99232096 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Wed, 28 Nov 2012 22:31:13 +0100 Subject: [PATCH] Fixed #19366 -- Prevented GEOSIndexError when comparing geometries Thanks Craig de Stigter for the report and collaboration on the patch. --- django/contrib/gis/geos/mutable_list.py | 23 +++++++++++-------- django/contrib/gis/geos/tests/test_geos.py | 15 ++++++++++++ .../gis/geos/tests/test_mutable_list.py | 9 ++++++++ docs/ref/contrib/gis/geos.txt | 11 +++++++++ 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/django/contrib/gis/geos/mutable_list.py b/django/contrib/gis/geos/mutable_list.py index 820cdfa5a47..0418282bfe9 100644 --- a/django/contrib/gis/geos/mutable_list.py +++ b/django/contrib/gis/geos/mutable_list.py @@ -149,27 +149,30 @@ class ListMixin(object): return self def __eq__(self, other): - for i in range(len(self)): + olen = len(other) + for i in range(olen): try: c = self[i] == other[i] - except IndexError: - # must be other is shorter + except self._IndexError: + # self must be shorter return False if not c: return False - return True + return len(self) == olen def __lt__(self, other): - slen = len(self) - for i in range(slen): + olen = len(other) + for i in range(olen): try: c = self[i] < other[i] - except IndexError: - # must be other is shorter - return False + except self._IndexError: + # self must be shorter + return True if c: return c - return slen < len(other) + elif other[i] < self[i]: + return False + return len(self) < olen ### Public list interface Methods ### ## Non-mutating ## diff --git a/django/contrib/gis/geos/tests/test_geos.py b/django/contrib/gis/geos/tests/test_geos.py index 283daa47c0e..ec320f94ec9 100644 --- a/django/contrib/gis/geos/tests/test_geos.py +++ b/django/contrib/gis/geos/tests/test_geos.py @@ -451,6 +451,21 @@ class GEOSTest(unittest.TestCase, TestDataMixin): self.assertEqual(poly.wkt, Polygon(*tuple(r for r in poly)).wkt) self.assertEqual(poly.wkt, Polygon(*tuple(LinearRing(r.tuple) for r in poly)).wkt) + def test_polygon_comparison(self): + p1 = Polygon(((0, 0), (0, 1), (1, 1), (1, 0), (0, 0))) + p2 = Polygon(((0, 0), (0, 1), (1, 0), (0, 0))) + self.assertTrue(p1 > p2) + self.assertFalse(p1 < p2) + self.assertFalse(p2 > p1) + self.assertTrue(p2 < p1) + + p3 = Polygon(((0, 0), (0, 1), (1, 1), (2, 0), (0, 0))) + p4 = Polygon(((0, 0), (0, 1), (2, 2), (1, 0), (0, 0))) + self.assertFalse(p4 < p3) + self.assertTrue(p3 < p4) + self.assertTrue(p4 > p3) + self.assertFalse(p3 > p4) + def test_multipolygons(self): "Testing MultiPolygon objects." prev = fromstr('POINT (0 0)') diff --git a/django/contrib/gis/geos/tests/test_mutable_list.py b/django/contrib/gis/geos/tests/test_mutable_list.py index 675505f0f93..988d8417a2b 100644 --- a/django/contrib/gis/geos/tests/test_mutable_list.py +++ b/django/contrib/gis/geos/tests/test_mutable_list.py @@ -363,6 +363,7 @@ class ListMixinTest(unittest.TestCase): pl, ul = self.lists_of_len() self.assertEqual(pl, ul, 'cmp for equal') + self.assertFalse(ul == pl + [2], 'cmp for not equal') self.assertTrue(pl >= ul, 'cmp for gte self') self.assertTrue(pl <= ul, 'cmp for lte self') self.assertTrue(ul >= pl, 'cmp for self gte') @@ -377,6 +378,14 @@ class ListMixinTest(unittest.TestCase): self.assertTrue(ul < pl + [2], 'cmp') self.assertTrue(ul <= pl + [2], 'cmp') + # Also works with a custom IndexError + ul_longer = ul + [2] + ul_longer._IndexError = TypeError + ul._IndexError = TypeError + self.assertFalse(ul_longer == pl) + self.assertFalse(ul == ul_longer) + self.assertTrue(ul_longer > ul) + pl[1] = 20 self.assertTrue(pl > ul, 'cmp for gt self') self.assertTrue(ul < pl, 'cmp for self lt') diff --git a/docs/ref/contrib/gis/geos.txt b/docs/ref/contrib/gis/geos.txt index eb20b1f4115..7d7c32781cb 100644 --- a/docs/ref/contrib/gis/geos.txt +++ b/docs/ref/contrib/gis/geos.txt @@ -656,6 +656,17 @@ is returned instead. Returns the number of interior rings in this geometry. +.. admonition:: Comparing Polygons + + Note that it is possible to compare ``Polygon`` objects directly with ``<`` + or ``>``, but as the comparison is made through Polygon's + :class:`LineString`, it does not mean much (but is consistent and quick). + You can always force the comparison with the :attr:`~GEOSGeometry.area` + property:: + + >>> if poly_1.area > poly_2.area: + >>> pass + Geometry Collections ====================