[Django] #21021: Default geom_type attribute for GeometryWidget should be "Geometry", not "Unknown"
Django
"Unknown"
--------------------------------------+------------------------
Reporter: leplatrem | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: GIS | Version: 1.6-beta-1
Severity: Normal | Keywords: minor
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------
The default value for geom_type is "Unknown" whereas we expect it to be
"Geometry".
Indeed, by default we rely on GDAL to set the geom_type attribute, and it
returns an unexpected value :
{{{
>>> from django.contrib.gis import gdal
>>> str(gdal.OGRGeomType('GEOMETRY'))
'Unknown'
}}}
In fields, we override explicitly his value, so the problem is hidden.
https://github.com/django/django/blob/1.6b2/django/contrib/gis/forms/fields.py#L42
But IMHO the widget should behave nicely with default values, for example
instantiated like this :
{{{
class BigMapWidget(BaseGeometryWidget):
map_height = 1200
class GeomForm(forms.Form):
def __init__(self, *args, **kwargs):
super(GeomForm, self).__init__(*args, **kwargs)
self.fields['geom'].widget = BigMapWidget()
}}}
Of course, this problem **only** applies to **generic Geometry** field
types, and those are commonly used.
--
Ticket URL: <https://code.djangoproject.com/ticket/21021>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
"Unknown"
Reporter: leplatrem | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: GIS | 1.6-beta-1
Severity: Normal | Resolution:
Keywords: minor | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:1>
Django
"Unknown"
Reporter: leplatrem | Owner:
Type: | EricBoersma
Cleanup/optimization | Status: assigned
Component: GIS | Version:
Severity: Normal | 1.6-beta-1
Keywords: minor | Resolution:
Has patch: 0 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by EricBoersma):
* status: new => assigned
* owner: nobody => EricBoersma
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:2>
Django
"Unknown"
Reporter: leplatrem | Owner:
Type: | EricBoersma
Cleanup/optimization | Status: assigned
Component: GIS | Version:
Severity: Normal | 1.6-beta-1
Keywords: minor | Resolution:
Needs tests: 0 | checkin
Easy pickings: 0 | Needs documentation: 0
| Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin
Comment:
Eric, don't forget to link to your PR from the ticket and check "Has
patch" so it's picked up for review.
Here's [https://github.com/django/django/pull/1568 Eric's PR]. It looks
good to me but I'll let Claude or something more familiar with GeoDjango
do a final review.
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:3>
Django
"Unknown"
Reporter: leplatrem | Owner:
Type: | EricBoersma
Cleanup/optimization | Status: assigned
Component: GIS | Version:
Severity: Normal | 1.6-beta-1
Keywords: minor | Resolution:
Needs tests: 0 | Needs documentation: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by claudep):
* stage: Ready for checkin => Accepted
Comment:
Eric, can you explain where the `102` number comes from in your patch? I'm
not sure changing the OGR semantic is a good idea. I think we'd rather
solve this in the forms/widgets layer. But I'm open to any other argument.
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:4>
Django
"Unknown"
Reporter: leplatrem | Owner:
Type: | EricBoersma
Cleanup/optimization | Status: assigned
Component: GIS | Version:
Severity: Normal | 1.6-beta-1
Keywords: minor | Resolution:
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timo):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:5>
Django
"Unknown"
Reporter: Mathieu Leplatre | Owner: Olivier
Type: | Tabone
Cleanup/optimization | Status: assigned
Component: GIS | Version:
| 1.6-beta-1
Severity: Normal | Resolution:
Keywords: minor | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Olivier Tabone):
* cc: olivier.tabone@… (added)
* owner: EricBoersma => Olivier Tabone
Comment:
claim the ticket, working on it during duth sprint.
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:6>
Django
"Unknown"
Reporter: Mathieu Leplatre | Owner: Olivier
Type: | Tabone
Cleanup/optimization | Status: assigned
| 1.6-beta-1
Severity: Normal | Resolution:
Keywords: minor | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Olivier Tabone):
* needs_better_patch: 1 => 0
* has_patch: 1 => 0
Comment:
Looking at GDAL documentation, there is no OGR type matching 'POLYGON'.
[http://www.gdal.org/ogr__core_8h.html#a800236a0d460ef66e687b7b65610f12a
enum OGRwkbGeometryType]
Hence the '102' magic number in the patch does not match OGR
specification.
I agree with Claude we should not change OGR semantics. returning
wkbUnknown for an empty geometry seems correct according to GDAL.
removed 'has patch' flag because the pull request has been closed quite a
long time ago by tim.
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:7>
Django
"Unknown"
Reporter: Mathieu Leplatre | Owner: (none)
Type: | Status: new
Cleanup/optimization | Version:
Severity: Normal | Resolution:
Keywords: minor | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Olivier Tabone):
* status: assigned => new
* owner: Olivier Tabone => (none)
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:8>
Django
"Unknown"
Reporter: Mathieu Leplatre | Owner: Giannis
Type: | Adamopoulos
Cleanup/optimization | Status: assigned
Component: GIS | Version:
| 1.6-beta-1
Severity: Normal | Resolution:
Keywords: minor | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Giannis Adamopoulos):
* owner: (none) => Giannis Adamopoulos
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:9>
Django
"Unknown"
Reporter: Mathieu Leplatre | Owner: Giannis
Type: | Adamopoulos
Cleanup/optimization | Status: assigned
| 1.6-beta-1
Severity: Normal | Resolution:
Keywords: minor | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* has_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
[https://github.com/django/django/pull/13718 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:10>
Django
"Unknown"
Reporter: Mathieu Leplatre | Owner: Giannis
Type: | Adamopoulos
Cleanup/optimization | Status: assigned
| 1.6-beta-1
Severity: Normal | Resolution:
Keywords: minor | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:11>
Django
"Unknown"
Reporter: Mathieu Leplatre | Owner: Giannis
Type: | Adamopoulos
Component: GIS | Version:
| 1.6-beta-1
Severity: Normal | Resolution: fixed
Keywords: minor | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"7603036bd0629d5f9e58f8da2418170070357f62" 7603036b]:
{{{
#!CommitTicketReference repository=""
revision="7603036bd0629d5f9e58f8da2418170070357f62"
Fixed #21021 -- Changed BaseGeometryWidget's default geometry type to
'Geometry'.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21021#comment:12>