[Django] #34406: Add support for curved geometries in GeoDjango
Django
-------------------------------------+-------------------------------------
Reporter: Fabien Le | Owner: nobody
Frapper |
Type: New | Status: new
feature |
Component: GIS | Version: 4.1
Severity: Normal | Keywords: geodjango gdal
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I tried ingesting curved geometries in a `GeometryField` in GeoDjango.
At first I encountered some errors as these are not officially supported
in GeoDjango, but I noticed that `gdal` is able to go from a geometry to
another using converters :
- Currently supported geometries
https://github.com/django/django/blob/main/django/contrib/gis/gdal/geometries.py#L727
- Corresponding geometries in `gdal`
https://gdal.org/doxygen/ogr__core_8h.html#a800236a0d460ef66e687b7b65610f12a
I successfully ingested the following geometries for a project :
- 9: "CompoundCurve"
- 10: "CurvePolygon"
- 11: "MultiCurve"
- 12: "MultiSurface"
----
Below is a code snippet I used, subclassing `OGRGeometry` and
`OGRGeomType` in order to bypass existing GeoDjango validation to add
support for more geometries :
- Link to the snippet with syntax highlighting
https://gist.github.com/fabienheureux/2dd4dad8f7c4fedef708154eea1470f9
- It mainly adds keys for new geometries and classes inheriting for
`OGRGeometry` to handle these in GeoDjango
- It shows how we could make these geometries backward compatible (from
curved geometries to more standard geometries) using a simple call to
existing gdal methods
**Is there a reason why it is not supported at the moment in GeoDjango ?
Would you consider a pull request adding support for these geometries ?**
I could suggest a patch based on the snippet above, but I am not sure how
to treat the `transform` part of it.
Should we keep this part ?
It seems that Postgis can handle these polygons
https://postgis.net/docs/using_postgis_dbmanagement.html#CircularString
----
Here is the same snippet but without syntax highlighting
{{{
from django.contrib.gis.gdal.geometries import GEO_CLASSES, OGRGeometry
from django.contrib.gis.gdal.geomtype import OGRGeomType
from django.contrib.gis.gdal.libgdal import lgdal
from django.contrib.gis.gdal.prototypes import ds as capi
from django.contrib.gis.gdal.prototypes import geom as geom_api
class ExtendedOGRGeometry(OGRGeometry):
def __init__(self, geom_input, srs=None):
try:
super().__init__(geom_input, srs)
except KeyError:
if (
not isinstance(geom_input, self.ptr_type)
and self.geom_type.num not in gdal_transform.keys()
):
raise
self.__class__ = EXTENDED_GEO_CLASSES[self.geom_type.num]
@property
def geom_type(self):
"Return the Type for this Geometry."
return ExtendedOGRGeomType(geom_api.get_geom_type(self.ptr))
class CurvePolygon(ExtendedOGRGeometry):
pass
class CompoundCurve(ExtendedOGRGeometry):
pass
class MultiSurface(ExtendedOGRGeometry):
pass
class MultiCurve(ExtendedOGRGeometry):
pass
EXTENDED_GEO_CLASSES = {
**GEO_CLASSES,
9: CompoundCurve,
10: CurvePolygon,
11: MultiCurve,
12: MultiSurface,
}
class ExtendedOGRGeomType(OGRGeomType):
# Copy paste of original types dictionnary from GeoDjango
implementation
#
https://github.com/django/django/blob/main/django/contrib/gis/gdal/geomtype.py#L9
_types = {
0: "Unknown",
1: "Point",
2: "LineString",
3: "Polygon",
4: "MultiPoint",
5: "MultiLineString",
6: "MultiPolygon",
7: "GeometryCollection",
100: "None",
101: "LinearRing",
102: "PointZ",
1 + OGRGeomType.wkb25bit: "Point25D",
2 + OGRGeomType.wkb25bit: "LineString25D",
3 + OGRGeomType.wkb25bit: "Polygon25D",
4 + OGRGeomType.wkb25bit: "MultiPoint25D",
5 + OGRGeomType.wkb25bit: "MultiLineString25D",
6 + OGRGeomType.wkb25bit: "MultiPolygon25D",
7 + OGRGeomType.wkb25bit: "GeometryCollection25D",
# Extended geometry types
9: "CompoundCurve",
10: "CurvePolygon",
11: "MultiCurve",
12: "MultiSurface",
}
# New bindings to existing GDAL methods
force_to_polygon = geom_output(lgdal.OGR_G_ForceToPolygon, [c_void_p])
force_to_multi_polygon = geom_output(lgdal.OGR_G_ForceToMultiPolygon,
[c_void_p])
force_to_line = geom_output(lgdal.OGR_G_ForceToLineString, [c_void_p])
force_to_multi_line = geom_output(lgdal.OGR_G_ForceToMultiLineString,
[c_void_p])
# The functions below need to be called on the corresponding geometry type
# before saving it in the database to prevent errors in geodjango.
gdal_transform = {
9: force_to_line,
10: force_to_polygon,
11: force_to_multi_line,
12: force_to_multi_polygon,
}
def ingest_curved_geometry(geom_ptr):
transform = gdal_transform[geom.geom_type.num]
geom = ExtendedOGRGeometry(transform(geom_api.clone_geom(geom_ptr)))
# FIXME: for a yet unknown reason, the initial SRID is not kept when
using ExtendedOGRGeometry
geom.srid = 3857
geom.transform(4326)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34406>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Fabien Le Frapper | Owner: nobody
Type: New feature | Status: new
Component: GIS | Version: 4.1
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz):
At first, I'd say that we are open to extend OGR recognition of those
types. However, I wonder if the missing GEOS type counterpart may prevent
proper usage of those new types in GeoDjango. At the very least, we can
recognize these new types and error out with a proper error message if we
don't support them in the framework. Accepting on that base.
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:1>
Django
-----------------------------------+------------------------------------
Type: New feature | Status: new
Component: GIS | Version: 4.1
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+------------------------------------
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:2>
Django
-----------------------------------+------------------------------------
Reporter: Fabien Le Frapper | Owner: nobody
Type: New feature | Status: new
Component: GIS | Version: 4.1
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+------------------------------------
Comment (by Fabien Le Frapper):
Replying to [comment:1 Claude Paroz]:
> At first, I'd say that we are open to extend OGR recognition of those
types. However, I wonder if the missing GEOS type counterpart may prevent
proper usage of those new types in GeoDjango. At the very least, we can
recognize these new types and error out with a proper error message if we
don't support them in the framework. Accepting on that base.
Thanks for your reply.
Do we have existing geodjango features that are gdal-only ?
Would a gdal-only contribution erroring in case of GEOS be accepted ?
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:3>
Django
Reporter: Fabien Le Frapper | Owner: Fabien Le
| Frapper
Type: New feature | Status: assigned
Component: GIS | Version: 4.1
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Fabien Le Frapper):
* owner: nobody => Fabien Le Frapper
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Fabien Le Frapper | Owner: Fabien Le
| Frapper
Type: New feature | Status: assigned
Component: GIS | Version: 4.1
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
----
----
Here is the full snippet
{{{#!python
self.__class__ = EXTENDED_GEO_CLASSES[self.geom_type.num]
class CurvePolygon(ExtendedOGRGeometry):
pass
class CompoundCurve(ExtendedOGRGeometry):
pass
class MultiSurface(ExtendedOGRGeometry):
pass
class MultiCurve(ExtendedOGRGeometry):
pass
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:5>
Django
Old description:
New description:
I tried ingesting curved geometries in a `GeometryField` in GeoDjango.
At first I encountered some errors as these are not officially supported
in GeoDjango, but I noticed that `gdal` is able to go from a geometry to
another using converters :
- Currently supported geometries
https://github.com/django/django/blob/main/django/contrib/gis/gdal/geometries.py#L727
- Corresponding geometries in `gdal`
https://gdal.org/doxygen/ogr__core_8h.html#a800236a0d460ef66e687b7b65610f12a
I successfully ingested the following geometries for a project :
- 9: "CompoundCurve"
- 10: "CurvePolygon"
- 11: "MultiCurve"
- 12: "MultiSurface"
----
Below is a code snippet I used, subclassing `OGRGeometry` and
`OGRGeomType` in order to bypass existing GeoDjango validation to add
support for more geometries :
----
self.__class__ = EXTENDED_GEO_CLASSES[self.geom_type.num]
class CurvePolygon(ExtendedOGRGeometry):
pass
class CompoundCurve(ExtendedOGRGeometry):
pass
class MultiSurface(ExtendedOGRGeometry):
pass
class MultiCurve(ExtendedOGRGeometry):
pass
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Fabien Le Frapper | Owner: Fabien Le
| Frapper
Type: New feature | Status: assigned
Component: GIS | Version: 4.1
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Anthony Ricaud (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Fabien Le Frapper | Owner: Fabien Le
| Frapper
Type: New feature | Status: assigned
Component: GIS | Version: 4.1
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Smith):
Here is a link to GDAL's RFC for "Curve geometries"
[https://gdal.org/development/rfc/rfc49_curve_geometries.html RFC 49]
This required the following C API changes. I think we should make a
decision on each of these (should Django implement them) as part of this
ticket.
- OGR_GT_xxxx (for Geometry Type)
- OGRErr OGR_G_ExportToIsoWkb( OGRGeometryH, OGRwkbByteOrder, unsigned
char*) -- Proposed in [https://github.com/django/django/pull/17696 PR]
- OGRErr OGR_G_ExportToIsoWkt( OGRGeometryH, char ) -- Proposed in
[https://github.com/django/django/pull/17696 PR]
- OGRGeometryH OGR_G_Value( OGRGeometryH, double dfDistance )
- int OGR_G_HasCurveGeometry( OGRGeometryH, int bLookForNonLinear )
- OGRGeometryH OGR_G_GetLinearGeometry( OGRGeometryH hGeom, double
dfMaxAngleStepSizeDegrees, char papszOptions)
- OGRGeometryH OGR_G_GetCurveGeometry( OGRGeometryH hGeom, char
papszOptions )
- void OGRSetNonLinearGeometriesEnabledFlag(int bFlag)
- int OGRGetNonLinearGeometriesEnabledFlag()
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:8>
Django
Reporter: Fabien Le Frapper | Owner: David Smith
Type: New feature | Status: assigned
Component: GIS | Version: 4.1
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by David Smith):
* owner: Fabien Le Frapper => David Smith
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:9>
Django
-----------------------------------+---------------------------------------
Reporter: Fabien Le Frapper | Owner: David Smith
Type: New feature | Status: assigned
Component: GIS | Version: 4.1
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
-----------------------------------+---------------------------------------
Changes (by David Smith):
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:10>
Django
-----------------------------------+---------------------------------------
Reporter: Fabien Le Frapper | Owner: David Smith
Type: New feature | Status: assigned
Keywords: geodjango gdal | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
-----------------------------------+---------------------------------------
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
* version: 4.1 => dev
Comment:
Setting patch flags per latest reviews from Claude and myself.
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:11>
Django
-----------------------------------+---------------------------------------
Reporter: Fabien Le Frapper | Owner: David Smith
Type: New feature | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-----------------------------------+---------------------------------------
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:12>
Django
-----------------------------------+---------------------------------------
Reporter: Fabien Le Frapper | Owner: David Smith
Type: New feature | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+---------------------------------------
Just looking at some of the other C API changes that were made in GDAL at
the time this was implemented. These two items I suggest that we don't
implement.
- void OGRSetNonLinearGeometriesEnabledFlag(int bFlag)
- int OGRGetNonLinearGeometriesEnabledFlag()
says:
> If they don't want to test the geometry type and explicitly calling the
conversion function, they can call
OGRSetNonLinearGeometriesEnabledFlag(FALSE) (the default value is TRUE,
i.e. non-linear geometries can be returned). In which case, they will be
transformed into their closest linear geometry, by doing linear
approximation, with OGR_G_ForceTo().
Given [https://github.com/django/django/pull/18007 PR#18007] proposes to
implement the geometry curve checking (via `has_curve`) and the linear
conversion method I think that probably gives us enough coverage here.
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:13>
Django
-----------------------------------+---------------------------------------
Reporter: Fabien Le Frapper | Owner: David Smith
Type: New feature | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
-----------------------------------+---------------------------------------
* needs_better_patch: 0 => 1
Setting to patch needs improvement until next week where I'll review
answers from David (so this is no longer listed in the patch needing
review list).
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:14>
Django
-----------------------------------+---------------------------------------
Reporter: Fabien Le Frapper | Owner: David Smith
Type: New feature | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
-----------------------------------+---------------------------------------
Changes (by Natalia Bidart):
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:15>
Django
-----------------------------------+---------------------------------------
Reporter: Fabien Le Frapper | Owner: David Smith
Type: New feature | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
-----------------------------------+---------------------------------------
Changes (by Natalia Bidart):
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:16>
Django
-----------------------------------+---------------------------------------
Reporter: Fabien Le Frapper | Owner: David Smith
Type: New feature | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+---------------------------------------
In [changeset:"04adff9f98a93a546d7b4d7453b80965233d22a3" 04adff9f]:
{{{#!CommitTicketReference repository=""
revision="04adff9f98a93a546d7b4d7453b80965233d22a3"
Refs #34406 -- Added support for GDAL curved geometries.
Co-authored-by: Fabien Le Frapper <con...@fabienlefrapper.me>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:17>
Django
-----------------------------------+---------------------------------------
Reporter: Fabien Le Frapper | Owner: David Smith
Type: New feature | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+---------------------------------------
* cc: Claude Paroz (added)
Comment:
Claude, David: I have merged the latest PR from David and now I wonder
what's left to do for this ticket? Could either of you, please, summarize
next steps for future contributors?
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:18>
Django
-----------------------------------+---------------------------------------
Reporter: Fabien Le Frapper | Owner: David Smith
Type: New feature | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Resolution:
Keywords: geodjango gdal | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+---------------------------------------
Thanks for the patch and the merge!
I'm not sure we should go further with this issue. Maybe `OGR_G_Value`?
David?
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:19>
Django
-----------------------------------+---------------------------------------
Reporter: Fabien Le Frapper | Owner: David Smith
Component: GIS | Version: dev
Severity: Normal | Resolution: fixed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+---------------------------------------
* resolution: => fixed
* status: assigned => closed
Comment:
I think we can close this issue. We could always consider `OGR_G_Value` as
part of #35058
--
Ticket URL: <https://code.djangoproject.com/ticket/34406#comment:20>