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 line/line-segment type? #99

Closed
kestred opened this issue Mar 13, 2017 · 7 comments
Closed

Add a line/line-segment type? #99

kestred opened this issue Mar 13, 2017 · 7 comments

Comments

@kestred
Copy link

kestred commented Mar 13, 2017

Does it make sense to have a separate type for a Line, like https://github.com/paulmach/go.geo/blob/master/line.go (in that it is treated as a LineString for GeoJSON/WKT)?

In particular, I would like this feature for strictly typed conversions using Postgres's builtin Line Segment type (using https://github.com/sfackler/rust-postgres). I had heard that it might be the plan to represent geo::types as traits; in which case this could either be its own trait or an implementation of LineString. (Aside: if you've written up the current direction of georust somewhere, I'd love a link!).

@frewsxcv
Copy link
Member

Is the distinction here that a Line is a series of Coordinates instead of a series of Points?

@TeXitoi
Copy link
Contributor

TeXitoi commented Mar 14, 2017

I think here a line is a segment, composed of 2 coordinates.

@frewsxcv
Copy link
Member

frewsxcv commented Mar 18, 2017

(Aside: if you've written up the current direction of georust somewhere, I'd love a link!)

I don't have anything in writing really. I haven't done a great job at establishing a direction for rust-geo, other than saying it's a place for geospatial types and algorithms. I don't do much geospatial work anymore, and have been pretty busy with some other Rust projects, so if anyone has good ideas for the direction of this crate, let me know.

I had heard that it might be the plan to represent geo::types as traits; in which case this could either be its own trait or an implementation of LineString.

Traits would be cool, and I have a pull request open experimenting with them, but with the direction I was heading, it would have made the ergonomics significantly worse than they currently are, and I wasn't a fan of that. That's not to say traits aren't the way forward, they just might require more thinking.

Does it make sense to have a separate type for a Line, like https://github.com/paulmach/go.geo/blob/master/line.go (in that it is treated as a LineString for GeoJSON/WKT)?

I'm not really sure about this. For now, I've been very loosely following version 1.1.0 of the Simple Features spec

which has a data model that looks like:

screen shot 2017-03-18 at 4 11 44 pm

the latest version of the spec (v 1.2.1) has a data model that looks like:

screen shot 2017-03-18 at 4 11 51 pm

In that model, you'll see a Line type, which presumably corresponds to the type you're talking about. Maybe it makes sense to start modeling after v1.2.1? Does it make sense to follow a spec? Does it make sense to add all these new types? These are questions I don't know.

Sorry, that's not much of an answer, would appreciate any feedback here

@kestred
Copy link
Author

kestred commented Mar 20, 2017

Disclosure: I'm also pretty new to GIS programming at this point, so take these opinions with some grains of salt.

Modelling the library according to the Simple Features specification seems like the best approach to take, because most GIS Databases support most (or some) of the specification.
I would definitely like to implement (or see implemented) an updated to the v1.2+ version of the spec (incorporating Z/M coordinates, open/closed, etc).

Following that plan, it doesn't make sense, at least initially, to include the Line/LinearRing/Triangle types because the specification describes them as non-instantiable; but also:

that the SFA-CA implementation standard assumes that a system handles these two types by means of added functionality that is not defined by the SFA-SQL


I'm thinking the final outcome might be:

  • Remove Coordinate type (should just be Point)
  • Have traits for Curve, Surface, MultiSurface, and MultiCurve
  • Have "core" types for Point, LineString, Polygon, MultiPoint, MultiPolygon, and MultiLineString
  • Have some extension with types (possibly in one of the other crates, or in a submodule) for Line, LinearRing since these types have distinct types in Spatial Schema (https://www.iso.org/standard/26012.html)

Still not sure whether Geometry should be a type (enum like you have now) or trait --- but this should probably just be based on the library's ergonomic needs.

@urschrei
Copy link
Member

Some thoughts:

I'm in favour of compliance with a spec, but with an emphasis on "core" types, as above.
Some other things I'd like to see:

With the addition of the above, I feel like the library would cover most people's day-to-day needs (I haven't done much linear referencing or transformation, so I assume it's somewhat niche – this is obviously wrong)

  • FFI!

I'm also time-constrained, and will be until at least July (working on this project is a form of distraction for me), but I'm also conscious that we've made a lot of progress in the last 9 months or so, and I'd like to see this continue / expand. We could do a lot more in terms of finding new contributors, and the current quality of the code base and the reviews (@frewsxcv and @TeXitoi are fantastic) makes this a hugely valuable learning experience.

@frewsxcv
Copy link
Member

frewsxcv commented Jun 1, 2017

A PR was opened last week with an implementation for Line: #118

@frewsxcv
Copy link
Member

This was completed in #118

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

No branches or pull requests

4 participants