[Django] #21003: BaseGeometryWidget is not idempotent
Django
--------------------------------------+------------------------
Reporter: leplatrem | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: GIS | Version: 1.6-beta-1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------
Currently, the geometry data is serialized as wkt :
https://github.com/django/django/blob/1.6b2/django/contrib/gis/forms/widgets.py#L69
https://github.com/django/django/blob/1.6b2/django/contrib/gis/templates/gis/openlayers.html#L20
And is read back here:
https://github.com/django/django/blob/1.6b2/django/contrib/gis/forms/fields.py#L95
But transform() will fail, because the instantiated Geometry won't have
any projection associated (wkt does not carry srid).
```
GEOSException at ...
Calling transform() with no SRID set is not supported
```
https://github.com/django/django/blob/1.6b2/django/contrib/gis/forms/fields.py#L96
There are several possible solutions :
* Use EWKT instead of WKT
* Add srid=self.map_srid parameter to fromstr() when instantiating
GEOSGeometry()
* Concatenate 'SRID=%s' % self.map_srid to the received data (this is
currently being done like this in JS in openlayers widget code
https://github.com/django/django/blob/1.6b2/django/contrib/gis/static/gis/js/OLMapWidget.js#L263-L273)
--
Ticket URL: <https://code.djangoproject.com/ticket/21003>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: leplatrem | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: GIS | 1.6-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
What about:
{{{
diff --git a/django/contrib/gis/forms/fields.py
b/django/contrib/gis/forms/fields.py
index 2fb8b54..04a19fd 100644
--- a/django/contrib/gis/forms/fields.py
+++ b/django/contrib/gis/forms/fields.py
@@ -87,6 +87,8 @@ class GeometryField(forms.Field):
# Only do a geographic comparison if both values are available
if initial and data:
data = fromstr(data)
+ if not data.srid and self.srid:
+ data.srid = self.srid
data.transform(initial.srid)
# If the initial value was not added by the browser, the
geometry
# provided may be slightly different, the first time it is
saved.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21003#comment:1>
Django
Reporter: leplatrem | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: GIS | 1.6-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by leplatrem):
Hum, unless I don't get it, it's wrong.
In your proposition, you assign the database field srid to the data coming
for the widget, whereas you should be using the widget's srid !
Something like this :
::
if not data.srid:
data.srid = self.widget.map_srid
If you want to see another approach, for example, in django-leaflet, I did
that to fix that problem:
https://github.com/makinacorpus/django-
leaflet/blob/add_leaflet_form_widget_and_admin/leaflet/templates/leaflet/widget.html#L34
Which is a consequence of the lack of modularity mentionned in #20998:
because in this particular case, what is transmitted between the field and
and the widgets is EWKT !
--
Ticket URL: <https://code.djangoproject.com/ticket/21003#comment:2>
Django
Reporter: leplatrem | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: GIS | 1.6-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by leplatrem):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/21003#comment:3>
Django
Reporter: leplatrem | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: GIS | 1.6-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by leplatrem):
Since SRID is ignored while parsing GeoJSON ("crs" attribute), this bug
prevents from using other serialization format than e/wkt.
Indeed, data coming from widget is read without assigning srid during
deserialization. If the serialized format cannot carry it, the resulting
geometry has no srid. Which fails at first transform() operation...
--
Ticket URL: <https://code.djangoproject.com/ticket/21003#comment:4>
Django
Reporter: leplatrem | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: GIS | 1.6-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by claudep):
Here's a slightly different patch. I didn't like that we had to
reconstruct a Geometry object at two different places in the same file.
And I prefer to test the srid after having created the object, because you
never know if the client has provided a value containing a parsable srid
through a clever thing. Criticism welcome!
--
Ticket URL: <https://code.djangoproject.com/ticket/21003#comment:5>
Django
Reporter: leplatrem | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: GIS | 1.6-beta-1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by leplatrem):
Indeed, it's a lot better to use to_python() ! Great, thanks !
--
Ticket URL: <https://code.djangoproject.com/ticket/21003#comment:6>
Django
Reporter: leplatrem | Owner: nobody
Type: | Status: closed
Cleanup/optimization | Version:
Component: GIS | 1.6-beta-1
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Claude Paroz <claude@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"dd656073ad80d0e8ec9e9dfea30da75a660d759c"]:
{{{
#!CommitTicketReference repository=""
revision="dd656073ad80d0e8ec9e9dfea30da75a660d759c"
Fixed #21003 -- Ensured geometry widget return value has SRID
Thanks Mathieu Leplatre for the report and initial patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21003#comment:7>
Django
Reporter: leplatrem | Owner: nobody
Type: | Status: closed
Cleanup/optimization | Version:
Component: GIS | 1.6-beta-1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Claude Paroz <claude@…>):
In [changeset:"0514fbb2f37b530e46f54e205cb8c9d722e142d6"]:
{{{
#!CommitTicketReference repository=""
revision="0514fbb2f37b530e46f54e205cb8c9d722e142d6"
[1.6.x] Fixed #21003 -- Ensured geometry widget return value has SRID
Thanks Mathieu Leplatre for the report and initial patch.
Backport of dd656073ad from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21003#comment:8>