Opened 6 weeks ago
Last modified 2 days ago
#36036 assigned New feature
Support the 4th dimension (M) in the GEOS API
Reported by: | Andrew | Owned by: | Andrew |
---|---|---|---|
Component: | GIS | Version: | dev |
Severity: | Normal | Keywords: | GIS 4d GEOS |
Cc: | Claude Paroz, Mariusz Felisiak, David Smith | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The GEOS API supports a 4th dimension, M, as well as PostGIS. Unfortunately Django does not and would be great if it could. I've started a WIP branch to see what this could look like.
Change History (24)
comment:1 by , 6 weeks ago
Keywords: | GEOS added; GESO removed |
---|
comment:2 by , 6 weeks ago
Cc: | added |
---|---|
Resolution: | → needsinfo |
Status: | new → closed |
comment:3 by , 6 weeks ago
I asked "what is 4d geos" and AI Overview says, ""4D Geos" refers to a field within geoscience that incorporates the element of time into spatial data analysis, essentially creating a "four-dimensional" view of geological processes or environmental changes by analyzing data across both space and time; this often involves using advanced GIS (Geographic Information Systems) tools to visualize and model these dynamic changes over time, making it particularly useful for studying things like land use changes, geological formations evolving over time, or environmental impacts across different periods."
I'm not a GeoDjango user, but this doesn't appear to be a controversial feature that requires a discussion. The changes appear straightforward and it doesn't seem useful to require users to implement this functionality as a third-party package or a subclass in every project that requires it.
comment:4 by , 6 weeks ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Patch needs improvement: | set |
Resolution: | needsinfo |
Status: | closed → new |
Triage Stage: | Unreviewed → Accepted |
Version: | 4.2 → dev |
I do agree with Tim. We strive (in longer term) to replicate as much as possible the GEOS API, and this could be the next evolution in that trend.
comment:5 by , 4 weeks ago
Sorry for the silence over the holidays. Would it be worthwhile for me to get my existing branch reviewable as PR with tests, docs, etc.? I'm not sure the best process to getting an issue like this merged in. I'm happy to help however is best but as Tim and Claude mentioned this is helping GeoDjango support more of the GEOS API and the primary use case for this 4th dimension if for a time component of a geometry.
comment:6 by , 4 weeks ago
Andrew: yes, please! assign the issue to yourself and polish the PR. The guide for contributing can be found here. Let me know if you have any questions!
comment:7 by , 4 weeks ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:9 by , 4 weeks ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
comment:10 by , 4 weeks ago
Description: | modified (diff) |
---|
comment:11 by , 4 weeks ago
It appears the test runner is using an out of date GEOS lib without GEOSGeomGetM which was added in 3.12.0. Is it a reasonable expectation to have geos updated to support this work?
GEOSHasM is also missing on the test runner GEOS version and was added three years ago in this PR so it seems to be pretty out of date.
comment:12 by , 4 weeks ago
Django is keeping compatibility with GIS libs until ~5years after they've been released. So Django will still support GEOS < 3.12 during the 2-3 next years (3.11 was released on July 2022).
In any case, your patch must take that into account and add conditional support for the M dimension depending on the installed version. If this is too difficult to achieve with reasonable efforts, I guess we'll have to postpone that issue for later (and set the Someday/Maybe triage state).
Regarding the test runners, it might be indeed interesting to have at least one config using the latest Ubuntu stable (see https://code.djangoproject.com/wiki/CI for current configs).
comment:13 by , 4 weeks ago
That makes sense. I'll look into conditional support but if it seems like a significant effort I can update the state of this issue. I'll also look into adding another test runner config too. Thanks
comment:14 by , 4 weeks ago
In any case, I think conditionally adding the GEOSHasM
support could be done (in its own PR/commit).
comment:15 by , 4 weeks ago
I've added the conditional support for GEOSHasM
in a separate commit and got CI to pass. Is the separate commit sufficient or are you thinking there should be separate PRs? I wasn't sure how best these could be separated into individual PRs in a meaningful way.
At the moment CI does not run the 4D tests because we don't have a test runner configured to use GEOS > 3.12, but I have been able to successfully run the tests locally on an up to date GEOS version. I wasn't sure where we'd config the test runners to also test on a later version.
comment:16 by , 4 weeks ago
A separate PR would be nice, as we can see test passing for that commit only.
Other than that, I have the feeling that tests are missing for x/y/m scenarios (M dimension without Z). But that could be discussed directly on the PR.
comment:17 by , 4 weeks ago
Yeah it will require a little more refactor to support 3d where the third dim is M not Z, but feel doable.
Is this PR the level of isolation you were expecting for adding conditional support for GEOSHasM
?
comment:18 by , 4 weeks ago
Patch needs improvement: | unset |
---|
Yes, it's the patch granularity I was expecting, thanks!
Even if it's not the final patch, I'm unchecking "Patch needs improvement" so that the patch goes to the review queue.
comment:19 by , 3 weeks ago
I wanted to follow up with getting the separate PR approved to merge. We've been waiting on some input but not sure when we might expect to get final feedback. I'm happy to update however reviewers feel is best.
Additionally I've been struggling with the WKT writer to have it correctly write XYM coordinates. I'm wondering if it would be acceptable to have an initial PR merged that only supports the M dimension for 4D coordinates (XYZM). I'm fairly confidence the the primary use case for the M dimension will be for XYZM and XYM would be a relatively niche use case.
Thanks again for all y'alls guidance and input on this issue.
comment:20 by , 6 days ago
I've addressed PR feedback in the small GEOSHasM PR. Is there anything else I can do help get it merged?
Also still curious if it'd be acceptable to first patch to support XYZM coordinates and then separately add XYM should that need arise?
comment:21 by , 5 days ago
Patch needs improvement: | set |
---|
comment:22 by , 3 days ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:24 by , 2 days ago
Has patch: | unset |
---|---|
Triage Stage: | Ready for checkin → Accepted |
Hello Andrew, thank you for taking the time to create this ticket.
I've been trying to search for examples and "real life" use cases for this feature, I can see that Postgis supports the 4th-dimension but I'm not finding practical use cases to justify including this into Django. Basically we need to determine whether this would be used by niche use cases, or if this is truly something that applies to the broader ecosystem, since Django is a framework designed to offer robust and accurate solutions for common scenarios.
I'm adding as CC a few experts in this topic to see what they think, but I think the best path forward at this point is to start a new forum conversation proposing this idea. The right category would be GeoDjango. In there you'll reach a wider audience and get extra feedback. More information in the documented guidelines for requesting features.