GEOSGeometry.transform() silently no-ops when GDAL isn't available or srid is None
|Reported by:||rcoup||Owned by:||rcoup|
|Severity:||Keywords:||transform geos gdal|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
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
- .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.
Change History (6)
Changed 6 years ago by rcoup
comment:1 Changed 6 years ago by rcoup
- Cc jbronn added
- Has patch set
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Triage Stage changed from Unreviewed to Accepted
comment:2 Changed 6 years ago by rcoup
- Owner changed from nobody to rcoup
- Status changed from new to assigned
Changed 6 years ago by jbronn
comment:3 Changed 6 years ago by jbronn
- Resolution set to fixed
- Status changed from assigned to closed