Opened 6 years ago

Closed 2 years ago

#9806 closed Bug (fixed)

GeometryField crashes contrib.gis.admin

Reported by: ingenieroariel Owned by: jbronn
Component: GIS Version: 1.0
Severity: Normal Keywords: admin, gis, GeometryField
Cc: ingenieroariel@…, dane.springmeyer@…, slinkp@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I tried to load this basic model in django.contrib.gis admin and I ended up with a:
Exception Value: Invalid OGR String Type "GEOMETRY"

class SimpleGeo(models.Model):
     geom = models.GeometryField(null=True, blank=True)

admin.site.register(SimpleGeo, admin.GeoModelAdmin)

I have inserted two objects, a Point and a Polygon and when using the shell,
I got the correct geom_type in both cases and dir listed the right attributes and methods for each field.

The line that triggers the error is:
'geom_type' : OGRGeomType(db_field._geom), in django/contrib/gis/admin/options.py#L84

I think one of two things should happen:

  • 1. Either the admin defaults to showing a plain text field whenever that happens or
  • 2. (pony alert) we find a way to look for the _geom of the instance and not the class.

Full traceback below:

Traceback:
File "/Library/Python/2.5/site-packages/django/core/handlers/base.py" in get_response
  86.                 response = callback(request, *callback_args, **callback_kwargs)
File "/Library/Python/2.5/site-packages/django/contrib/admin/sites.py" in root
  158.                 return self.model_page(request, *url.split('/', 2))
File "/Library/Python/2.5/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  44.         response = view_func(request, *args, **kwargs)
File "/Library/Python/2.5/site-packages/django/contrib/admin/sites.py" in model_page
  177.         return admin_obj(request, rest_of_url)
File "/Library/Python/2.5/site-packages/django/contrib/admin/options.py" in __call__
  197.             return self.change_view(request, unquote(url))
File "/Library/Python/2.5/site-packages/django/db/transaction.py" in _commit_on_success
  238.                 res = func(*args, **kw)
File "/Library/Python/2.5/site-packages/django/contrib/admin/options.py" in change_view
  573.         ModelForm = self.get_form(request, obj)
File "/Library/Python/2.5/site-packages/django/contrib/admin/options.py" in get_form
  277.         return modelform_factory(self.model, **defaults)
File "/Library/Python/2.5/site-packages/django/forms/models.py" in modelform_factory
  318.                               'formfield_callback': formfield_callback})
File "/Library/Python/2.5/site-packages/django/forms/models.py" in __new__
  180.                                       opts.exclude, formfield_callback)
File "/Library/Python/2.5/site-packages/django/forms/models.py" in fields_for_model
  147.         formfield = formfield_callback(f)
File "/Library/Python/2.5/site-packages/django/contrib/gis/admin/options.py" in formfield_for_dbfield
  58.             kwargs['widget'] = self.get_map_widget(db_field)
File "/Library/Python/2.5/site-packages/django/contrib/gis/admin/options.py" in get_map_widget
  76.         class OLMap(self.widget):
File "/Library/Python/2.5/site-packages/django/contrib/gis/admin/options.py" in OLMap
  84.                       'geom_type' : OGRGeomType(db_field._geom),
File "/Library/Python/2.5/site-packages/django/contrib/gis/gdal/geomtype.py" in __init__
  29.                 raise OGRException('Invalid OGR String Type "%s"' % type_input)

Attachments (6)

hack.diff (625 bytes) - added by ingenieroariel 6 years ago.
Dirty hack that avoids the crash.
9806.diff (1.2 KB) - added by ingenieroariel 6 years ago.
Patch, tested in Firefox 3 on MacOSX
9806.2.diff (5.5 KB) - added by ingenieroariel 6 years ago.
Now allows edition of already populated geometries.
9806.3.diff (5.6 KB) - added by tbnorth 5 years ago.
path updated for rev 13856
9806.4.diff (10.2 KB) - added by slinkp 5 years ago.
9806.5.diff (16.6 KB) - added by slinkp 4 years ago.
patch that allows adding as well as editing geometries in admin UI for GeometryField

Download all attachments as: .zip

Change History (32)

comment:1 Changed 6 years ago by jbronn

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to jbronn
  • Patch needs improvement unset

Yeah, I don't thing the geographic admin ever supported GeometryField -- we need this ticket. Patches welcome on whatever method you choose to implement, as I won't be able to get to this for a while (takes a lot less of my time to review a patch already written).

comment:2 Changed 6 years ago by ingenieroariel

Here is a workaround that makes the admin load up but does not render the feature.
In the following file
gs/admin/options.py

Before:

params = {
                  ...
                 'geom_type' : OGRGeomType(db_field._geom),
                  ...
}

After

        #OGRGeomType crashes for 'GEOMETRY' type.
        #It'd be better if we found a way to get the geom type of the instance, but there
        #does not seem to be an easy way to accomplish that.
        try:
           geom_type=OGRGeomType(db_field._geom)
        except:
           geom_type=OGRGeomType('Unknown')
        params = {
                  ...
                 'geom_type' : geom_type,
                  ...
        }

Changed 6 years ago by ingenieroariel

Dirty hack that avoids the crash.

comment:3 Changed 6 years ago by ingenieroariel

Attached is a less than ideal solution that avoids the crash.

OGR Already has a type for a generic GeometryField and it is called 'Unknown'.
I have the following questions:

  • Why does geodjango call it 'GEOMETRY' is it because PostGIS calls it that way?
  • Do we do a try catch or do we specify the 'GEOMETRY' to 'Unknown' equivalence?
  • How do we handle collections on GeometryFields?

I am gonna start looking at how we can make rendering more intelligent for Unknown fields, right now it works for points but not for polygons or multipolygons.

comment:4 Changed 6 years ago by ingenieroariel

The error that appears in the admin that causes no geometry to be rendered after passing geom_type = 'Unknown' is the following:

controls[i] is undefined
OpenLayers.js
Line 374:
this.controls=this.controls.concat(controls);for(var i=0;i<controls.length;i++){var element=document.createElement("div");var textNode=document.createTextNode(" ");controls[i].panel_div=element;if(controls[i].title!=""){controls[i].panel_div.title=controls[i].title;}

Changed 6 years ago by ingenieroariel

Patch, tested in Firefox 3 on MacOSX

comment:5 Changed 6 years ago by ingenieroariel

  • Has patch set
  • Owner changed from jbronn to anonymous
  • Patch needs improvement set
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

That's the simplest fix I could come up with.

Status changed to accepted based on Justin's 'we need this ticket' statement

Any thoughts?

comment:6 Changed 6 years ago by ingenieroariel

  • Owner changed from anonymous to jbronn
  • Status changed from assigned to new

comment:7 Changed 6 years ago by jbronn

  • Status changed from new to assigned

Patch is a very good start; the template JS re-working to render the right controls is what needs work to get this fixed.

Changed 6 years ago by ingenieroariel

Now allows edition of already populated geometries.

comment:8 Changed 6 years ago by springmeyer

  • Cc dane.springmeyer@… added

comment:9 Changed 6 years ago by johanneswilm

latest patch doesn't appy cleanly to 1.0.2 and even doing it by hand, the line:

else: self.params['collection_type'] = OGRGeomType(db_field._geom.replace('MULTI', ''))

creates problems, because "db_field" is not defined (it is in options.py, but not in widgets.py).

I've checked the trunk-version, but couldn't find it there either.

comment:10 Changed 6 years ago by jbronn

(In [9985]) Maintenance refactor of the GDAL (OGR) ctypes interface. Changes include:

  • All C API method explictly called from their prototype module, no longer imported via *.
  • Applied DRY to C pointer management, classes that do so subclass from GDALBase.
  • OGRGeometry: Added from_bbox class method (patch from Christopher Schmidt) and kml property.
  • SpatialReference: Now initialize with SetFromUserInput (initialization is now more simple and flexible); removed duplicate methods.
  • Envelope: Added expand_to_include method and now allow same coordinates for lower left and upper right points. Thanks to Paul Smith for tickets and patches.
  • OGRGeomType: Now treat OGC 'Geometry' type as 'Unknown'.

Fixed #9855, #10368, #10380. Refs #9806.

comment:11 Changed 6 years ago by jbronn

  • milestone set to 1.1

comment:12 Changed 6 years ago by jbronn

  • milestone changed from 1.1 to 1.2

comment:13 Changed 5 years ago by jbronn

  • milestone changed from 1.2 to 1.3

Changed 5 years ago by tbnorth

path updated for rev 13856

comment:14 Changed 5 years ago by tbnorth

I've attached a new version of the patch which works against rev 13856.

For some reason MULTI* geometries are not being edited, I suspect it's something trivial. They are viewed though, and non-MULTI are viewed and editable, so the GeometryField is much more usable.

comment:15 Changed 5 years ago by slinkp

Patch works for me against Django 1.2.3, at least against points. I haven't tried with any MULTI*, dont' have any of those handy.

Changed 5 years ago by slinkp

comment:16 Changed 5 years ago by slinkp

I've just attached 9806.4.diff which, at least here, allows me to edit existing multipolygons.
Applies cleanly against Django 1.2.3.

This is a big improvement for me, but it still doesn't allow setting the geometry on a new instance of a model with a GeometryField.
This is because I don't know how to sensibly initialize draw_ctl in that case - we need an appropriate OpenLayers.Handler subclass, and I don't know what that would be. Don't see anything promising in the OL docs.

Not sure what to do here. We could just assume that people want MultiPolygons, which would be the most flexible,
but also kind of stupid in a pretty common case where the field is a GeometryField but the user just wants to enter a point.

comment:17 Changed 5 years ago by slinkp

I wonder if it would be possible to add a separate dropdown widget which allows the user to specify what kind of geometry they want.
This could be done entirely in javascript, all it would need to do is update the draw_ctl (and presumably remove existing features).

Changed 4 years ago by slinkp

patch that allows adding as well as editing geometries in admin UI for GeometryField

comment:18 Changed 4 years ago by slinkp

  • Needs tests set

Just added 9806.5.diff.
This allows adding geometries to GeometryFields. I punted on allowing arbitrary types, and assume that for new GeometryFields, the user wants to add polygons.

NOTE this requires OpenLayers trunk of at least revision r10972 ! This is because there was a relevant openlayers bug that just got fixed:
http://trac.osgeo.org/openlayers/ticket/2706
I presume this fix will be in OpenLayers 2.11, but that's not out - I have no idea of the timeline there.

This also includes my patch for #14886 which allows overriding the wms format type (default is image/jpg) as well as passing arbitrary other params for the wms URL.
I can remove that part of the patch if this patch seems worth pursuing.

comment:19 Changed 4 years ago by slinkp

  • Cc slinkp@… added

comment:20 Changed 4 years ago by jbronn

  • milestone changed from 1.3 to 1.4

comment:21 Changed 4 years ago by lukeplant

  • Severity set to Normal
  • Type set to Bug

comment:22 Changed 4 years ago by jacob

  • milestone 1.4 deleted

Milestone 1.4 deleted

comment:2 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:3 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:4 Changed 2 years ago by dyve

Any progress on this? It seems a solution was close, but didn't quite work. I keep bumping into this one with any GIS enabled app that allows GeometryField (quite common in my line of work).

comment:5 Changed 2 years ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In e6f5b7eacd32afb892c486a5b0994f7f11170868:

Fixed #9806 -- Allowed editing GeometryField with OpenLayersWidget

Thanks Paul Winkler for the initial patch.

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