Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a spatial function to convert WKT to Geo-JSON #406

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Andy2003
Copy link
Collaborator

This PR adds a new spatial function spatial.convert.wktToGeoJson to convert WKT to Geo-JSON.

return spatial.convert.wktToGeoJson("MULTIPOLYGON(((15.3 60.2, 15.3 60.4, 15.7 60.4, 15.7 60.2, 15.3 60.2)))") as json

returns an object like

{
  "type": "MultiPolygon",
   "coordinates", [[[
      [15.3, 60.2],
      [15.3, 60.4],
      [15.7, 60.4],
      [15.7, 60.2],
      [15.3, 60.2]
]]]

@Andy2003 Andy2003 requested a review from jexp May 29, 2024 17:28
@Andy2003 Andy2003 force-pushed the feature/add-mapper-for-geo-json branch from ff6a85c to 62b3b7a Compare May 30, 2024 11:17
@Andy2003 Andy2003 force-pushed the feature/add-mapper-for-geo-json branch from 62b3b7a to 415522f Compare May 31, 2024 14:46
Copy link
Contributor

@craigtaverner craigtaverner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a useful function. But it also feels incomplete, missing a few geometry types, and with only one test for one type. I think we need both:

  • Unit test for the GeoJsonUtils class that tests all types
  • More spatial functions tests for at least a geometrycollection with various types

This is also an opportunity to make a function spatial.asGeoJson that takes a geometry, or a node, and produces GeoJSON. That does not need to be in the PR, but seems like most of the work is already done for that since your implementation makes an intermediate Geometry.

}

private static List<?> getCoordinates(Geometry geometry) {
if (geometry instanceof Point point) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are missing types here, like MultiPoint, MultiLineString, MultiPolygon. Should be easy enough to add these.

Copy link
Collaborator Author

@Andy2003 Andy2003 May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No all these types are covered by inheritance:
image

@Andy2003 Andy2003 force-pushed the master branch 5 times, most recently from 6844765 to bee28a4 Compare June 20, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants