Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #13788 -- `GEOSGeometry.transform` no longer silently no-ops wh…

…en GDAL isn't available. Thanks, Rob Coup.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@15025 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 5fddfda5593d0f8c244f640ae63a17035ade0e3e 1 parent df4cb60
@jbronn jbronn authored
View
48 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 @@ def transform(self, ct, clone=False):
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):
View
108 django/contrib/gis/geos/tests/test_geos.py
@@ -852,6 +852,114 @@ def test23_transform(self):
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.
View
4 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
View
14 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``
---------
View
11 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 <ref-gdal>` 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::
View
14 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``.

0 comments on commit 5fddfda

Please sign in to comment.
Something went wrong with that request. Please try again.