Opened 7 years ago

Closed 17 months ago

Last modified 8 months ago

#27674 closed Cleanup/optimization (fixed)

Deprecate GeoModelAdmin and OSMGeoAdmin

Reported by: Claude Paroz Owned by: Mariusz Felisiak
Component: GIS Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The geometry fields now have default map widgets, so the need of gis-specific ModelAdmin classes is therefore less relevant.

However, the fact that ModelAdmin.formfield_for_dbfield is not documented does not help, so I'd suggest to document it first.

Change History (20)

comment:1 Changed 7 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

Could you describe the relevance of formfield_for_dbfield in a bit more detail?

comment:2 Changed 7 years ago by Claude Paroz

Sure. The typical example is to be sure all geometry widgets use a specific map widget, or specific map widget attributes.
With formfield_overrides, you'd have to specify all field types:

class MyModelAdmin(admin.ModelAdmin):
    formfield_overrides = {
        models.PointField: {'widget': MyCustomMapWidget},
        models.PolygonField: {'widget': MyCustomMapWidget},
        models.LineField: {'widget': MyCustomMapWidget},
        models.MultiPointField: {'widget': MyCustomMapWidget},
        ...
    }

which is not nice if you have many geometry field types in your project. I think it's a case where formfield_for_dbfield is handy:

class MyModelAdmin(admin.ModelAdmin):
    def formfield_for_dbfield(self, db_field, request, **kwargs):
        if isinstance(db_field, models.GeometryField) and db_field.dim < 3:
            kwargs['widget'] = OSMWidget(default_lon=151, default_lat=-33)
            return db_field.formfield(**kwargs)
        else:
            return super(GeoModelAdmin, self).formfield_for_dbfield(db_field, request, **kwargs)

An alternative would be to still provide a GIS admin utility (subclass or mixin) which sets the same map widget for all geometry fields, like the current GeoModelAdmin.get_map_widget(). Basically the current code without all the boiler plate code copying class attributes to widget attributes.

comment:3 Changed 7 years ago by Tim Graham

Another idea: I like to see this sort of issue solved at the form level, if possible, to allow reusing the solution outside the admin.

comment:4 Changed 3 years ago by Giannis Adamopoulos

Owner: changed from nobody to Giannis Adamopoulos
Status: newassigned

comment:5 Changed 3 years ago by Giannis Adamopoulos

comment:6 Changed 3 years ago by Mariusz Felisiak

Has patch: set

comment:7 Changed 3 years ago by Mariusz Felisiak

Needs documentation: set
Needs tests: set
Patch needs improvement: set

comment:8 Changed 2 years ago by Mariusz Felisiak

Needs documentation: unset
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:9 Changed 2 years ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 4555aa0a:

Fixed #27674 -- Deprecated GeoModelAdmin and OSMGeoAdmin.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@…>

comment:10 Changed 22 months ago by Nick Pope

While looking into #33372 I noticed that there are two OpenLayersWidget classes:

  • django.contrib.gis.admin.widgets.OpenLayersWidget - this is used by the deprecated GeoModelAdmin class and targets OpenLayers 2.x
  • django.contrib.gis.forms.widgets.OpenLayersWidget - this is used as a parent to OSMWidget (used by the new GISModelAdmin) and targets OpenLayers 3.x

Deprecation warnings were added for GeoModelAdmin and OSMGeoAdmin as part of this ticket but .admin.widgets.OpenLayersWidget has no warning. Do we need to add one?

Do we also need to warn about gis/admin/openlayers.html and gis/admin/osm.html templates going away? (Not sure whether we add a documentation note?)

comment:11 in reply to:  10 Changed 22 months ago by Mariusz Felisiak

Needs tests: unset
Resolution: fixed
Status: closednew
Triage Stage: Ready for checkinAccepted

Replying to Nick Pope:

While looking into #33372 I noticed that there are two OpenLayersWidget classes:

  • django.contrib.gis.admin.widgets.OpenLayersWidget - this is used by the deprecated GeoModelAdmin class and targets OpenLayers 2.x
  • django.contrib.gis.forms.widgets.OpenLayersWidget - this is used as a parent to OSMWidget (used by the new GISModelAdmin) and targets OpenLayers 3.x

Deprecation warnings were added for GeoModelAdmin and OSMGeoAdmin as part of this ticket but .admin.widgets.OpenLayersWidget has no warning. Do we need to add one?

Agreed, we can deprecate django.contrib.gis.admin.widgets.OpenLayersWidget in Django 4.1 and remove it in 5.0. Let's reopen.

Do we also need to warn about gis/admin/openlayers.html and gis/admin/osm.html templates going away? (Not sure whether we add a documentation note?)

As far as I'm concerned, it is not needed.

comment:12 Changed 22 months ago by Mariusz Felisiak

Has patch: unset

comment:13 Changed 21 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 44814509:

Refs #27674 -- Added tests for GISModelAdmin.gis_widget_kwargs.

comment:14 Changed 21 months ago by Mariusz Felisiak

Owner: Giannis Adamopoulos deleted
Status: newassigned

comment:15 Changed 21 months ago by Mariusz Felisiak

Status: assignednew

comment:16 Changed 17 months ago by Mariusz Felisiak

Has patch: set
Owner: set to Mariusz Felisiak
Status: newassigned

comment:17 Changed 17 months ago by GitHub <noreply@…>

In eeb0bb6:

Refs #27674 --- Deprecated django.contrib.gis.admin.OpenLayersWidget.

comment:18 Changed 17 months ago by Mariusz Felisiak

Resolution: fixed
Status: assignedclosed

comment:19 Changed 8 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 4982958:

Refs #27674 -- Removed GeoModelAdmin and OSMGeoAdmin per deprecation timeline.

comment:20 Changed 8 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In ce7b4f3:

Refs #27674 -- Removed django.contrib.gis.admin.OpenLayersWidget per deprecation timeline.

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