Ticket #13788: 13788_geos_transform.2.diff

django/contrib/gis/geos/geometry.py

@@ -4 +4 @@
 """
 # 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 +499 @@
         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):

django/contrib/gis/geos/tests/test_geos.py

@@ -852 +852 @@
             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
+        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)
+
+        # test warning is raised
+        try:
+            warnings.simplefilter('error', FutureWarning)
+
+            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', category=FutureWarning)
+
+        print "\nEND - expecting Warnings; safe to ignore.\n"
+
+    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.

docs/internals/deprecation.txt

@@ -136 +136 @@
           template variable, not an implied string. The new-style
           behavior is provided in the ``future`` template tag library.
 
+        * :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

docs/ref/contrib/gis/geos.txt

@@ -523 +523 @@
 
     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 +539 @@
 
     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``
 ---------
 

docs/ref/contrib/gis/install.txt

@@ -98 +98 @@
 .. 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 +274 @@
 
 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::
 

docs/releases/1.3-alpha-2.txt

@@ -95 +95 @@
     **if the value is not None**, and falls back to the previously used
     :setting:`MEDIA_URL` setting otherwise.
 
+GeoDjango
+---------
+
+  * 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``.
+
+
 The Django 1.3 roadmap
 ======================