Opened 18 months ago

Closed 18 months ago

Last modified 18 months ago

#20998 closed Cleanup/optimization (fixed)

Form GIS widgets render() lack of modularity

Reported by: leplatrem Owned by: nobody
Component: GIS Version: 1.6-beta-1
Severity: Normal Keywords: refactor
Cc: claudep Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When subclassing GIS form widgets, there should be a way to override geometry serialization, without having to duplicate reprojection behaviour.

https://github.com/django/django/blob/1.6b2/django/contrib/gis/forms/widgets.py#L37-L74

I propose to add some additional private methods:

  • _deserialize: string (wkt) to geos (geos)
  • _transform: from map widget to model projection
  • _serialize: geometry to string (wkt)

This would open doors to widgets that support other representation formats (geojson, topojson, ...)

Attachments (1)

refactor-gis-widgets-WIP.diff (7.9 KB) - added by leplatrem 18 months ago.
Draft of modifications intended

Download all attachments as: .zip

Change History (8)

Changed 18 months ago by leplatrem

Draft of modifications intended

comment:1 Changed 18 months ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

If the goal is to make the widgets extensible by third-party code, we need a public API rather than a private one.

comment:2 Changed 18 months ago by claudep

  • Has patch set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Many thanks for suggesting those improvements.

As discussed on IRC, my idea is to deprecate most of gis/admin as soon as the new form fields/widgets in gis/forms.py proved they can replace former functionality. So I wouldn't touch the gis/admin part, also for the fact that backwards compatibility is a concern for that code.

comment:3 Changed 18 months ago by claudep

  • Needs tests unset
  • Patch needs improvement unset
  • Type changed from Uncategorized to Cleanup/optimization

comment:4 Changed 18 months ago by Claude Paroz <claude@…>

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

In 102f26c92923d0e5e80de9b473f2343f76b04aa8:

Fixed #20998 -- Allow custom (de)serialization for GIS widgets

Thanks Mathieu Leplatre for the report and the initial patch.

comment:5 Changed 18 months ago by Claude Paroz <claude@…>

In 4e3794dd1fbc689dcbcc455f3458539d630a1ff3:

[1.6.x] Fixed #20998 -- Allow custom (de)serialization for GIS widgets

Thanks Mathieu Leplatre for the report and the initial patch.
Backport of 102f26c92 from master.

comment:6 Changed 18 months ago by Claude Paroz <claude@…>

In 973502c04704aac4742e2d996cb758acd5035d46:

Fixed gis test to run on non gis-enabled settings

Refs #20998.

comment:7 Changed 18 months ago by Claude Paroz <claude@…>

In 0c5786890875f2adbcfddd0056dc2b87a768aa1e:

[1.6.x] Fixed gis test to run on non gis-enabled settings

Refs #20998.
Backport of 973502c0 from master.

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