Opened 14 years ago

Closed 13 years ago

Last modified 12 years ago

#13788 closed (fixed)

GEOSGeometry.transform() silently no-ops when GDAL isn't available or srid is None

Reported by: Robert Coup Owned by: Robert Coup
Component: GIS Version: dev
Severity: Keywords: transform geos gdal
Cc: jbronn Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

GEOSGeometry.transform() uses the GDAL library to re-project geometries. If GDAL is not available however, it silently does nothing.

Geodjango-Users discussion thread: http://groups.google.com/group/geodjango/browse_thread/thread/788cafdbd592d617

Current behaviour if GDAL is unavailable:

  • .transform(clone=False) (the default) does nothing
  • .transform(clone=True) returns None

Also related:

  • .transform(clone=False) (the default) called on a geometry where .srid is None does nothing
  • .transform(clone=True) called on a geometry where .srid is None returns None

Since transform(clone=False) (the default) mutates the geometry, it should raise an exception if it can't do what the caller asked.

The attached patch:

  • short-circuits re-projection if the source and destination SRIDs match
  • makes .transform() raise a GEOSGeometry.Error if GDAL isn't available
  • adds tests around the new behaviour
  • adds a FutureWarning if the srid of the geometry is None or <0, pointing out that in 1.5 the behaviour will change an error will be raised.
  • adds some docs, including notes to the backwards-incompatibilities for 1.3

The backwards incompatible behaviour change is being introduced because:

  • .transform() no-op'ing if source & destination SRIDs don't match is NOT what the caller wants - it is in all cases a bug.
  • It's unlikely the user was manually checking either the return value (clone=True) or the geometry's srid (clone=False) and raising their own exception, the more common case would be to assert on HAS_GDAL.

Attachments (2)

13788_geos_transform.diff (10.1 KB ) - added by Robert Coup 14 years ago.
initial patch
13788_geos_transform.2.diff (10.0 KB ) - added by jbronn 13 years ago.
Updated

Download all attachments as: .zip

Change History (6)

by Robert Coup, 14 years ago

Attachment: 13788_geos_transform.diff added

initial patch

comment:1 by Robert Coup, 14 years ago

Cc: jbronn added
Has patch: set
Triage Stage: UnreviewedAccepted

My documentation/rest-fu isn't strong, so probably worth a look there, and i'm not sure whether the notes for the release are all in the right places :)

comment:2 by Robert Coup, 14 years ago

Owner: changed from nobody to Robert Coup
Status: newassigned

by jbronn, 13 years ago

Attachment: 13788_geos_transform.2.diff added

Updated

comment:3 by jbronn, 13 years ago

Resolution: fixed
Status: assignedclosed

(In [15025]) Fixed #13788 -- GEOSGeometry.transform no longer silently no-ops when GDAL isn't available. Thanks, Rob Coup.

comment:4 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top