diff --git a/django/contrib/gis/geos/geometry.py b/django/contrib/gis/geos/geometry.py index 34b126d777..a9d126481a 100644 --- a/django/contrib/gis/geos/geometry.py +++ b/django/contrib/gis/geos/geometry.py @@ -4,6 +4,7 @@ """ # Python, ctypes and types dependencies. import re +import warnings from ctypes import addressof, byref, c_double, c_size_t # super-class for mutable list behavior @@ -498,23 +499,40 @@ class GEOSGeometry(GEOSBase, ListMixin): instead. """ srid = self.srid - if gdal.HAS_GDAL and srid: - # Creating an OGR Geometry, which is then transformed. - g = gdal.OGRGeometry(self.wkb, srid) - g.transform(ct) - # Getting a new GEOS pointer - ptr = wkb_r().read(g.wkb) + + if ct == srid: + # short-circuit where source & dest SRIDs match if clone: - # User wants a cloned transformed geometry returned. - return GEOSGeometry(ptr, srid=g.srid) - if ptr: - # Reassigning pointer, and performing post-initialization setup - # again due to the reassignment. - capi.destroy_geom(self.ptr) - self.ptr = ptr - self._post_init(g.srid) + return self.clone() else: - raise GEOSException('Transformed WKB was invalid.') + return + + if (srid is None) or (srid < 0): + warnings.warn("Calling transform() with no SRID set does no transformation!", + stacklevel=2) + warnings.warn("Calling transform() with no SRID will raise GEOSException in v1.5", + FutureWarning, stacklevel=2) + return + + if not gdal.HAS_GDAL: + raise GEOSException("GDAL library is not available to transform() geometry.") + + # Creating an OGR Geometry, which is then transformed. + g = gdal.OGRGeometry(self.wkb, srid) + g.transform(ct) + # Getting a new GEOS pointer + ptr = wkb_r().read(g.wkb) + if clone: + # User wants a cloned transformed geometry returned. + return GEOSGeometry(ptr, srid=g.srid) + if ptr: + # Reassigning pointer, and performing post-initialization setup + # again due to the reassignment. + capi.destroy_geom(self.ptr) + self.ptr = ptr + self._post_init(g.srid) + else: + raise GEOSException('Transformed WKB was invalid.') #### Topology Routines #### def _topology(self, gptr): diff --git a/django/contrib/gis/geos/tests/test_geos.py b/django/contrib/gis/geos/tests/test_geos.py index c610093329..cc54becd3f 100644 --- a/django/contrib/gis/geos/tests/test_geos.py +++ b/django/contrib/gis/geos/tests/test_geos.py @@ -852,6 +852,114 @@ class GEOSTest(unittest.TestCase, TestDataMixin): self.assertAlmostEqual(trans.x, p.x, prec) self.assertAlmostEqual(trans.y, p.y, prec) + def test23_transform_noop(self): + """ Testing `transform` method (SRID match) """ + # transform() should no-op if source & dest SRIDs match, + # regardless of whether GDAL is available. + if gdal.HAS_GDAL: + g = GEOSGeometry('POINT (-104.609 38.255)', 4326) + gt = g.tuple + g.transform(4326) + self.assertEqual(g.tuple, gt) + self.assertEqual(g.srid, 4326) + + g = GEOSGeometry('POINT (-104.609 38.255)', 4326) + g1 = g.transform(4326, clone=True) + self.assertEqual(g1.tuple, g.tuple) + self.assertEqual(g1.srid, 4326) + self.assert_(g1 is not g, "Clone didn't happen") + + old_has_gdal = gdal.HAS_GDAL + try: + gdal.HAS_GDAL = False + + g = GEOSGeometry('POINT (-104.609 38.255)', 4326) + gt = g.tuple + g.transform(4326) + self.assertEqual(g.tuple, gt) + self.assertEqual(g.srid, 4326) + + g = GEOSGeometry('POINT (-104.609 38.255)', 4326) + g1 = g.transform(4326, clone=True) + self.assertEqual(g1.tuple, g.tuple) + self.assertEqual(g1.srid, 4326) + self.assert_(g1 is not g, "Clone didn't happen") + finally: + gdal.HAS_GDAL = old_has_gdal + + def test23_transform_nosrid(self): + """ Testing `transform` method (no SRID) """ + # raise a warning if SRID <0/None + import warnings + print "\nBEGIN - expecting Warnings; safe to ignore.\n" + + # test for do-nothing behaviour. + try: + # Keeping line-noise down by only printing the relevant + # warnings once. + warnings.simplefilter('once', UserWarning) + warnings.simplefilter('once', FutureWarning) + + g = GEOSGeometry('POINT (-104.609 38.255)', srid=None) + g.transform(2774) + self.assertEqual(g.tuple, (-104.609, 38.255)) + self.assertEqual(g.srid, None) + + g = GEOSGeometry('POINT (-104.609 38.255)', srid=None) + g1 = g.transform(2774, clone=True) + self.assert_(g1 is None) + + g = GEOSGeometry('POINT (-104.609 38.255)', srid=-1) + g.transform(2774) + self.assertEqual(g.tuple, (-104.609, 38.255)) + self.assertEqual(g.srid, -1) + + g = GEOSGeometry('POINT (-104.609 38.255)', srid=-1) + g1 = g.transform(2774, clone=True) + self.assert_(g1 is None) + + finally: + warnings.simplefilter('default', UserWarning) + warnings.simplefilter('default', FutureWarning) + + print "\nEND - expecting Warnings; safe to ignore.\n" + + + # test warning is raised + try: + warnings.simplefilter('error', FutureWarning) + warnings.simplefilter('ignore', UserWarning) + + g = GEOSGeometry('POINT (-104.609 38.255)', srid=None) + self.assertRaises(FutureWarning, g.transform, 2774) + + g = GEOSGeometry('POINT (-104.609 38.255)', srid=None) + self.assertRaises(FutureWarning, g.transform, 2774, clone=True) + + g = GEOSGeometry('POINT (-104.609 38.255)', srid=-1) + self.assertRaises(FutureWarning, g.transform, 2774) + + g = GEOSGeometry('POINT (-104.609 38.255)', srid=-1) + self.assertRaises(FutureWarning, g.transform, 2774, clone=True) + finally: + warnings.simplefilter('default', FutureWarning) + warnings.simplefilter('default', UserWarning) + + + def test23_transform_nogdal(self): + """ Testing `transform` method (GDAL not available) """ + old_has_gdal = gdal.HAS_GDAL + try: + gdal.HAS_GDAL = False + + g = GEOSGeometry('POINT (-104.609 38.255)', 4326) + self.assertRaises(GEOSException, g.transform, 2774) + + g = GEOSGeometry('POINT (-104.609 38.255)', 4326) + self.assertRaises(GEOSException, g.transform, 2774, clone=True) + finally: + gdal.HAS_GDAL = old_has_gdal + def test24_extent(self): "Testing `extent` method." # The xmin, ymin, xmax, ymax of the MultiPoint should be returned. diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 54f40d027a..508e6f2a6e 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -147,6 +147,10 @@ their deprecation, as per the :ref:`Django deprecation policy The ``supports_inactive_user`` variable is not checked any longer and can be removed. + * :meth:`~django.contrib.gis.geos.GEOSGeometry.transform` will raise + a :class:`~django.contrib.gis.geos.GEOSException` when called + on a geometry with no SRID value. + * 2.0 * ``django.views.defaults.shortcut()``. This function has been moved to ``django.contrib.contenttypes.views.shortcut()`` as part of the diff --git a/docs/ref/contrib/gis/geos.txt b/docs/ref/contrib/gis/geos.txt index 088ea6cdbe..95ba7e55e3 100644 --- a/docs/ref/contrib/gis/geos.txt +++ b/docs/ref/contrib/gis/geos.txt @@ -523,7 +523,9 @@ corresponding to the SRID of the geometry or ``None``. Requires GDAL. -.. method:: transform(ct, clone=False) +.. method:: GEOSGeometry.transform(ct, clone=False) + +.. versionchanged:: 1.3 Transforms the geometry according to the given coordinate transformation paramter (``ct``), which may be an integer SRID, spatial reference WKT string, @@ -537,6 +539,16 @@ is returned instead. Requires GDAL. +.. note:: + + Prior to 1.3, this method would silently no-op if GDAL was not available. + Now, a :class:`~django.contrib.gis.geos.GEOSException` is raised as + application code relying on this behavior is in error. In addition, + use of this method when the SRID is ``None`` or less than 0 now generates + a warning because a :class:`~django.contrib.gis.geos.GEOSException` will + be raised instead in version 1.5. + + ``Point`` --------- diff --git a/docs/ref/contrib/gis/install.txt b/docs/ref/contrib/gis/install.txt index fa8e34c268..a1485d1b54 100644 --- a/docs/ref/contrib/gis/install.txt +++ b/docs/ref/contrib/gis/install.txt @@ -98,8 +98,9 @@ Program Description Required .. admonition:: Install GDAL While :ref:`gdalbuild` is technically not required, it is *recommended*. - Some features of GeoDjango (including the :ref:`ref-layermapping` and the geographic - admin) depend on its functionality. + Important features of GeoDjango (including the :ref:`ref-layermapping`, + geometry reprojection, and the geographic admin) depend on its + functionality. .. note:: @@ -273,9 +274,9 @@ supports :ref:`GDAL's vector data ` capabilities [#]_. First download the latest GDAL release version and untar the archive:: - $ wget http://download.osgeo.org/gdal/gdal-1.7.2.tar.gz - $ tar xzf gdal-1.7.2.tar.gz - $ cd gdal-1.7.2 + $ wget http://download.osgeo.org/gdal/gdal-1.7.3.tar.gz + $ tar xzf gdal-1.7.3.tar.gz + $ cd gdal-1.7.3 Configure, make and install:: diff --git a/docs/releases/1.3.txt b/docs/releases/1.3.txt index 7992c95e5f..af40156410 100644 --- a/docs/releases/1.3.txt +++ b/docs/releases/1.3.txt @@ -528,6 +528,14 @@ statements manually. GeoDjango ~~~~~~~~~ -The :setting:`TEST_RUNNER` previously used to execute the GeoDjango test suite, -:func:`django.contrib.gis.tests.run_gis_tests`, is deprecated in favor of -the :class:`django.contrib.gis.tests.GeoDjangoTestSuiteRunner` class. + * The function-based :setting:`TEST_RUNNER` previously used to execute + the GeoDjango test suite, :func:`django.contrib.gis.tests.run_gis_tests`, + was deprecated for the class-bassed runner, + :class:`django.contrib.gis.tests.GeoDjangoTestSuiteRunner`. + + * Previously, calling :meth:`~django.contrib.gis.geos.GEOSGeometry.transform` + would silently do nothing when GDAL wasn't available. Now, + a :class:`~django.contrib.gis.geos.GEOSException` is properly raised + to indicate possible faulty application code. A warning is now raised + if :meth:`~django.contrib.gis.geos.GEOSGeometry.transform` is called when + the SRID of the geometry is less than 0 or ``None``.