Skip to content
This repository has been archived by the owner on May 16, 2022. It is now read-only.

Review #3

Closed
evetion opened this issue Jan 14, 2020 · 4 comments · Fixed by #5
Closed

Review #3

evetion opened this issue Jan 14, 2020 · 4 comments · Fixed by #5

Comments

@evetion
Copy link
Member

evetion commented Jan 14, 2020

Still writing things down, this post will be edited/extended!

This is gonna be a long post, not sure about the right format.

Thanks for taking the time to put together an proposal! SF is essential, although like most OGC standards, falls into the rabbit hole of prescriptive standards on top of a descriptive one. The latter is way more developer friendly and is what were doing here now. Therefore I would like to extend and dare I say improve the SF in this package, much like PostGIS has done, for usability. A nice implementation is also over at R.

Functions

I would like to add at least all the functions defined on the abstract geometry type, per SF, although we can play with the names.

  • dimension() : Integer
  • coordinateDimension() : Integer
  • spatialDimension() : Integer
  • geometry Type() : String
  • SRID() : Integer
  • envelope() : Geometry
  • asText() : String
  • asBinary () : Binary
  • isEmpty () : Boolean
  • isSimple() : Boolean
  • is3D() : Boolean
  • isMeasured() : Boolean
  • boundary () : Geometry

Not all of them need to be implemented, but fixing the spatial reference and an easy implementation for the bounding box seem essential to me. Speaking of functions that could use some actual algorithm, where will those be? The whole suite of DE-9IM functions? Pure Julia, or GEOS? Can you choose a backing engine (like a CPU/GPU switch?). Do we provide an area function (defined in SF for Polygons) or length. Where do these implementations belong?

Types

Screenshot 2020-01-14 at 22 00 22

As seen above, there are a lot more types than we now have defined. There are also intermediate types (Curve) or subtypes (Triangle). On top of this, the ISO SQL MM standard extends this, partly implemented and extended by PostGIS.

IO

Parsing WKT/WKB could be in another package that also implements this Interface? Same goes for GeoJSON and many other formats. We probably don't need a function for this.

@evetion evetion changed the title [WIP] Review Review Jan 15, 2020
@visr
Copy link
Member

visr commented Mar 15, 2020

Thanks for sharing your thought. Took me a while to get to this..

Functions

Regarding the list of functions you give, I think we should look carefully to see where it makes sense to follow SF, and where not. Some of them are already in there (dimension, coordinateDimension, spatialDimension, geometryType).

asText and asBinary are in there as wellknowntext and wellknownbinary, but I'm not 100% sure if they should stay to be honest. It would mean having to implement 'blessed' WKT/WKB types in this package. It seems preferable to have them as a concrete implementation that implement the interface.

isMeasured: I never come across m coordinates. Are they used much, and should we support them? Not having that would make is3D unneeded as well, since we can just check the number of dimensions.

isEmpty is covered by ngeom(g) == 0, though we could decide to add a Base.isempty fallback like that in this package.

For SRID, I'm not sure I want to include that at this point, for fear of not finishing the interface. Shall we put it on hold and think this through properly later, also how it would relate to GeoFormatTypes.jl?

isSimple, boundary and envelope are in a sense already spatial operations, and I'm not sure we should go there, because we'd be asking each library that conforms to the interface to implement it. I agree envelope (bounding box) may be appealing to have though, and we could make it optional and have a fallback here.

Speaking of functions that could use some actual algorithm, where will those be? The whole suite of DE-9IM functions? Pure Julia, or GEOS? Can you choose a backing engine (like a CPU/GPU switch?). Do we provide an area function (defined in SF for Polygons) or length. Where do these implementations belong?

I would consider these spatial operations, that don't make sense to implement seperately for each set of geometries. The interface will support converting geometries easily to LibGEOS.jl or GeometryBasics.jl, where DE-9IM implementations could reside.

Types

Good point, there are a lot more types in SF than we have right now. We have the GeoJSON subset. In my experience these are the most common and I have the most experience with them. We can always add more types to the interface, that will be easier than removing types. Also some may become covered by the issue you linked, https://github.com/JuliaGeometry/GeometryBasics.jl/issues/15. Or are there some of those that you consider important to add now?

IO

Parsing WKT/WKB could be in another package that also implements this Interface? Same goes for GeoJSON and many other formats. We probably don't need a function for this.

Ah yes, then it seems we agree. I think we should remove wellknowntext(geom) and wellknownbinary(geom) from the interface.

@evetion
Copy link
Member Author

evetion commented Mar 15, 2020

Some quick thoughts.

This spec doesn't have to implemented fully by other packages. Fallback options here would also include "not implemented" or even "ignored from spec".

In this way we can still include isMeasured. Its not often used, but a useful concept related to tables. Like a PointCloud, which is a feature, with geometry MultiPoint. Apart from the Z dimension, it can have a time dimension, which would be the M here. Ofcourse, there can be many Ms, so the only thing we need is a labeling system (like columnnames).

isEmpty is easy to implement and part of the spec, so lets include it.

SRID is essential for this spec in my opinion, could just be <:CoordinateReferenceSystemFormat from https://github.com/JuliaGeo/GeoFormatTypes.jl.

I understand your hesitation about functions and agree with them. However, can we just list these methods here and ask them to be implemented? In the "not implemented" error, we can even point them to say, LibGEOS. Same goes for WKT & WKB, and I agree they shouldn't be implemented here.

With regard to types, these are the simplest existing things to put into the spec. It's an ISO standard. So let's put them all in, including the the SQL-MM Part 3 ones (http://postgis.net/docs/using_postgis_dbmanagement.html#SQL_MM_Part3). With regards to GeometryBasics, I'd very much like them to implement this spec on top of their meshes. In that way I can easily convert, say a TIN or PolyhedralSurface to simple Polygons that will work in LibGEOS or GeoJSON.

Lastly, I really like to add a type for a boundingbox. I've done multiple for different packages now and they can't work together. PostGIS has done a similar thing: https://postgis.net/docs/Box2D.html. I can come up with a proposal?

@visr
Copy link
Member

visr commented Mar 15, 2020

I think it is better to start small, and expand from there, rather than trying to nail everything at once.

Regarding optional functions, I think in other interfaces such as Tables.jl some functions may be optional, but only in the sense that an interface fallback exists, that may be slower.

We could go for a pragmatic SRID, but may regret it later. If we add it now, it would need a good proposal.

Maybe you need to explain M to me sometime. Currently there is no way to label dimensions, but N dimensional points are supported. I guess in practice most will assume the first 2/3 to mean x, y(, z). So I'm also hesitant to throw it in there without a good proposal, showing how we can solve this in a general way.

You are right that we can easily add more geometry types. But they need to come together with the essential methods just like the other types, and at this point I'm not sure what they should be. I feel more confident setting this up for types that have 3 or more implementations. Writing the interface for types we don't have any of, may be a bit challenging and getting ahead of things at this point.

Would be great if you can come up with a proposal for adding a bounding box. I guess the simplest would be two points, one at the minimum of each dimension, and one at the maximum of each dimension?

@evetion
Copy link
Member Author

evetion commented Apr 5, 2020

Closed by #5

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants