From 9397d3add47f220249c8e543c79489618aaf50d6 Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Mon, 4 Sep 2017 02:19:37 +0100 Subject: [PATCH] Fixed #28558 -- Simplified code to remove OGRIndexError. The test is a regression for refs #4740 to show that the original fix of OGRIndexError is no longer needed. This is similar to the removal of GEOSIndexError in 197b1878105504b5ac7e399e1f52a6093c88baa3. --- django/contrib/gis/gdal/__init__.py | 6 +++--- django/contrib/gis/gdal/datasource.py | 11 +++-------- django/contrib/gis/gdal/error.py | 10 ---------- django/contrib/gis/gdal/feature.py | 11 +++-------- django/contrib/gis/gdal/geometries.py | 25 ++++--------------------- django/contrib/gis/gdal/layer.py | 10 ++++------ tests/gis_tests/gdal_tests/test_ds.py | 7 +++---- tests/gis_tests/gdal_tests/test_geom.py | 19 ++++++++++++++----- 8 files changed, 34 insertions(+), 65 deletions(-) diff --git a/django/contrib/gis/gdal/__init__.py b/django/contrib/gis/gdal/__init__.py index 6893f0daf7..6a0f5360d8 100644 --- a/django/contrib/gis/gdal/__init__.py +++ b/django/contrib/gis/gdal/__init__.py @@ -29,7 +29,7 @@ from django.contrib.gis.gdal.datasource import DataSource from django.contrib.gis.gdal.driver import Driver from django.contrib.gis.gdal.envelope import Envelope from django.contrib.gis.gdal.error import ( - GDALException, OGRIndexError, SRSException, check_err, + GDALException, SRSException, check_err, ) from django.contrib.gis.gdal.geometries import OGRGeometry from django.contrib.gis.gdal.geomtype import OGRGeomType @@ -42,6 +42,6 @@ from django.contrib.gis.gdal.srs import CoordTransform, SpatialReference __all__ = ( 'Driver', 'DataSource', 'CoordTransform', 'Envelope', 'GDALException', 'GDALRaster', 'GDAL_VERSION', 'OGRGeometry', 'OGRGeomType', - 'OGRIndexError', 'SpatialReference', 'SRSException', 'check_err', - 'gdal_version', 'gdal_full_version', + 'SpatialReference', 'SRSException', 'check_err', 'gdal_version', + 'gdal_full_version', ) diff --git a/django/contrib/gis/gdal/datasource.py b/django/contrib/gis/gdal/datasource.py index a399593b88..9890a5ab51 100644 --- a/django/contrib/gis/gdal/datasource.py +++ b/django/contrib/gis/gdal/datasource.py @@ -37,7 +37,7 @@ from ctypes import byref from django.contrib.gis.gdal.base import GDALBase from django.contrib.gis.gdal.driver import Driver -from django.contrib.gis.gdal.error import GDALException, OGRIndexError +from django.contrib.gis.gdal.error import GDALException from django.contrib.gis.gdal.layer import Layer from django.contrib.gis.gdal.prototypes import ds as capi from django.utils.encoding import force_bytes, force_text @@ -84,22 +84,17 @@ class DataSource(GDALBase): # Raise an exception if the returned pointer is NULL raise GDALException('Invalid data source file "%s"' % ds_input) - def __iter__(self): - "Allows for iteration over the layers in a data source." - for i in range(self.layer_count): - yield self[i] - def __getitem__(self, index): "Allows use of the index [] operator to get a layer at the index." if isinstance(index, str): layer = capi.get_layer_by_name(self.ptr, force_bytes(index)) if not layer: - raise OGRIndexError('invalid OGR Layer name given: "%s"' % index) + raise IndexError('invalid OGR Layer name given: "%s"' % index) elif isinstance(index, int): if 0 <= index < self.layer_count: layer = capi.get_layer(self._ptr, index) else: - raise OGRIndexError('index out of range') + raise IndexError('index out of range') else: raise TypeError('Invalid index type: %s' % type(index)) return Layer(layer, self) diff --git a/django/contrib/gis/gdal/error.py b/django/contrib/gis/gdal/error.py index 5c29c7c849..e394b6077c 100644 --- a/django/contrib/gis/gdal/error.py +++ b/django/contrib/gis/gdal/error.py @@ -14,16 +14,6 @@ class SRSException(Exception): pass -class OGRIndexError(GDALException, KeyError): - """ - This exception is raised when an invalid index is encountered, and has - the 'silent_variable_feature' attribute set to true. This ensures that - django's templates proceed to use the next lookup type gracefully when - an Exception is raised. Fixes ticket #4740. - """ - silent_variable_failure = True - - # #### GDAL/OGR error checking codes and routine #### # OGR Error Codes diff --git a/django/contrib/gis/gdal/feature.py b/django/contrib/gis/gdal/feature.py index 641e72dde7..9a398803fe 100644 --- a/django/contrib/gis/gdal/feature.py +++ b/django/contrib/gis/gdal/feature.py @@ -1,5 +1,5 @@ from django.contrib.gis.gdal.base import GDALBase -from django.contrib.gis.gdal.error import GDALException, OGRIndexError +from django.contrib.gis.gdal.error import GDALException from django.contrib.gis.gdal.field import Field from django.contrib.gis.gdal.geometries import OGRGeometry, OGRGeomType from django.contrib.gis.gdal.prototypes import ds as capi, geom as geom_api @@ -38,14 +38,9 @@ class Feature(GDALBase): elif 0 <= index < self.num_fields: i = index else: - raise OGRIndexError('index out of range') + raise IndexError('index out of range') return Field(self, i) - def __iter__(self): - "Iterate over each field in the Feature." - for i in range(self.num_fields): - yield self[i] - def __len__(self): "Return the count of fields in this feature." return self.num_fields @@ -116,5 +111,5 @@ class Feature(GDALBase): "Return the index of the given field name." i = capi.get_field_index(self.ptr, force_bytes(field_name)) if i < 0: - raise OGRIndexError('invalid OFT field name given: "%s"' % field_name) + raise IndexError('invalid OFT field name given: "%s"' % field_name) return i diff --git a/django/contrib/gis/gdal/geometries.py b/django/contrib/gis/gdal/geometries.py index 5f68ffa268..f2ca1345ed 100644 --- a/django/contrib/gis/gdal/geometries.py +++ b/django/contrib/gis/gdal/geometries.py @@ -44,9 +44,7 @@ from ctypes import byref, c_char_p, c_double, c_ubyte, c_void_p, string_at from django.contrib.gis.gdal.base import GDALBase from django.contrib.gis.gdal.envelope import Envelope, OGREnvelope -from django.contrib.gis.gdal.error import ( - GDALException, OGRIndexError, SRSException, -) +from django.contrib.gis.gdal.error import GDALException, SRSException from django.contrib.gis.gdal.geomtype import OGRGeomType from django.contrib.gis.gdal.libgdal import GDAL_VERSION from django.contrib.gis.gdal.prototypes import geom as capi, srs as srs_api @@ -566,12 +564,7 @@ class LineString(OGRGeometry): elif dim == 3: return (x.value, y.value, z.value) else: - raise OGRIndexError('index out of range: %s' % index) - - def __iter__(self): - "Iterate over each point in the LineString." - for i in range(self.point_count): - yield self[i] + raise IndexError('index out of range: %s' % index) def __len__(self): "Return the number of points in the LineString." @@ -618,17 +611,12 @@ class Polygon(OGRGeometry): "Return the number of interior rings in this Polygon." return self.geom_count - def __iter__(self): - "Iterate through each ring in the Polygon." - for i in range(self.geom_count): - yield self[i] - def __getitem__(self, index): "Get the ring at the specified index." if 0 <= index < self.geom_count: return OGRGeometry(capi.clone_geom(capi.get_geom_ref(self.ptr, index)), self.srs) else: - raise OGRIndexError('index out of range: %s' % index) + raise IndexError('index out of range: %s' % index) # Polygon Properties @property @@ -667,12 +655,7 @@ class GeometryCollection(OGRGeometry): if 0 <= index < self.geom_count: return OGRGeometry(capi.clone_geom(capi.get_geom_ref(self.ptr, index)), self.srs) else: - raise OGRIndexError('index out of range: %s' % index) - - def __iter__(self): - "Iterate over each Geometry." - for i in range(self.geom_count): - yield self[i] + raise IndexError('index out of range: %s' % index) def __len__(self): "Return the number of geometries in this Geometry Collection." diff --git a/django/contrib/gis/gdal/layer.py b/django/contrib/gis/gdal/layer.py index 384bc3a074..a2b86c46bd 100644 --- a/django/contrib/gis/gdal/layer.py +++ b/django/contrib/gis/gdal/layer.py @@ -3,9 +3,7 @@ from ctypes import byref, c_double from django.contrib.gis.gdal.base import GDALBase from django.contrib.gis.gdal.envelope import Envelope, OGREnvelope -from django.contrib.gis.gdal.error import ( - GDALException, OGRIndexError, SRSException, -) +from django.contrib.gis.gdal.error import GDALException, SRSException from django.contrib.gis.gdal.feature import Feature from django.contrib.gis.gdal.field import OGRFieldTypes from django.contrib.gis.gdal.geometries import OGRGeometry @@ -46,7 +44,7 @@ class Layer(GDALBase): # number of features because the beginning and ending feature IDs # are not guaranteed to be 0 and len(layer)-1, respectively. if index < 0: - raise OGRIndexError('Negative indices are not allowed on OGR Layers.') + raise IndexError('Negative indices are not allowed on OGR Layers.') return self._make_feature(index) elif isinstance(index, slice): # A slice was given @@ -87,8 +85,8 @@ class Layer(GDALBase): for feat in self: if feat.fid == feat_id: return feat - # Should have returned a Feature, raise an OGRIndexError. - raise OGRIndexError('Invalid feature id: %s.' % feat_id) + # Should have returned a Feature, raise an IndexError. + raise IndexError('Invalid feature id: %s.' % feat_id) # #### Layer properties #### @property diff --git a/tests/gis_tests/gdal_tests/test_ds.py b/tests/gis_tests/gdal_tests/test_ds.py index 535aba70fb..b0ebcba840 100644 --- a/tests/gis_tests/gdal_tests/test_ds.py +++ b/tests/gis_tests/gdal_tests/test_ds.py @@ -4,7 +4,6 @@ import unittest from django.contrib.gis.gdal import ( GDAL_VERSION, DataSource, Envelope, GDALException, OGRGeometry, - OGRIndexError, ) from django.contrib.gis.gdal.field import OFTInteger, OFTReal, OFTString @@ -84,7 +83,7 @@ class DataSourceTest(unittest.TestCase): self.assertEqual(source.driver, str(ds.driver)) # Making sure indexing works - with self.assertRaises(OGRIndexError): + with self.assertRaises(IndexError): ds[len(ds)] def test02_invalid_shp(self): @@ -120,9 +119,9 @@ class DataSourceTest(unittest.TestCase): self.assertIn(f, source.fields) # Negative FIDs are not allowed. - with self.assertRaises(OGRIndexError): + with self.assertRaises(IndexError): layer.__getitem__(-1) - with self.assertRaises(OGRIndexError): + with self.assertRaises(IndexError): layer.__getitem__(50000) if hasattr(source, 'field_values'): diff --git a/tests/gis_tests/gdal_tests/test_geom.py b/tests/gis_tests/gdal_tests/test_geom.py index 169857c0f2..aa1b371751 100644 --- a/tests/gis_tests/gdal_tests/test_geom.py +++ b/tests/gis_tests/gdal_tests/test_geom.py @@ -4,9 +4,10 @@ import unittest from binascii import b2a_hex from django.contrib.gis.gdal import ( - CoordTransform, GDALException, OGRGeometry, OGRGeomType, OGRIndexError, - SpatialReference, + CoordTransform, GDALException, OGRGeometry, OGRGeomType, SpatialReference, ) +from django.template import Context +from django.template.engine import Engine from ..test_data import TestDataMixin @@ -157,7 +158,7 @@ class OGRGeomTest(unittest.TestCase, TestDataMixin): self.assertEqual(ls.coords, linestr.tuple) self.assertEqual(linestr, OGRGeometry(ls.wkt)) self.assertNotEqual(linestr, prev) - with self.assertRaises(OGRIndexError): + with self.assertRaises(IndexError): linestr.__getitem__(len(linestr)) prev = linestr @@ -182,7 +183,7 @@ class OGRGeomTest(unittest.TestCase, TestDataMixin): for ls in mlinestr: self.assertEqual(2, ls.geom_type) self.assertEqual('LINESTRING', ls.geom_name) - with self.assertRaises(OGRIndexError): + with self.assertRaises(IndexError): mlinestr.__getitem__(len(mlinestr)) def test_linearring(self): @@ -232,6 +233,14 @@ class OGRGeomTest(unittest.TestCase, TestDataMixin): for r in poly: self.assertEqual('LINEARRING', r.geom_name) + def test_polygons_templates(self): + # Accessing Polygon attributes in templates should work. + engine = Engine() + template = engine.from_string('{{ polygons.0.wkt }}') + polygons = [OGRGeometry(p.wkt) for p in self.geometries.multipolygons[:2]] + content = template.render(Context({'polygons': polygons})) + self.assertIn('MULTIPOLYGON (((100', content) + def test_closepolygons(self): "Testing closing Polygon objects." # Both rings in this geometry are not closed. @@ -254,7 +263,7 @@ class OGRGeomTest(unittest.TestCase, TestDataMixin): if mp.valid: self.assertEqual(mp.n_p, mpoly.point_count) self.assertEqual(mp.num_geom, len(mpoly)) - with self.assertRaises(OGRIndexError): + with self.assertRaises(IndexError): mpoly.__getitem__(len(mpoly)) for p in mpoly: self.assertEqual('POLYGON', p.geom_name)