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

Initial geo-traits implementation #1011

Merged
merged 13 commits into from
Jun 21, 2023
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[workspace]
members = ["geo", "geo-types", "geo-postgis", "geo-test-fixtures", "jts-test-runner", "geo-bool-ops-benches"]
members = ["geo", "geo-types", "geo-traits", "geo-postgis", "geo-test-fixtures", "jts-test-runner", "geo-bool-ops-benches"]

[patch.crates-io]

Expand Down
19 changes: 19 additions & 0 deletions geo-traits/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
[package]
name = "geo-traits"
version = "0.1.0"
license = "MIT OR Apache-2.0"
repository = "https://github.com/georust/geo"
documentation = "https://docs.rs/geo-traits/"
readme = "../README.md"
keywords = ["gis", "geo", "geography", "geospatial"]
description = "Geospatial traits"
rust-version = "1.65"
Copy link
Member Author

Choose a reason for hiding this comment

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

Bumped this to 1.65 for this crate because the traits use GATs, which came in 1.65

edition = "2021"

[features]

[dependencies]
geo-types = "0.7.9"
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about making geo-types optional? In theory if someone wanted to just implement geo-traits for their own geometry types, they shouldn't need access to geo-types, I think

Copy link
Member

Choose a reason for hiding this comment

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

Hmm actually this depends on CoordTrait. Maybe that should live in this crate someday 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I think making geo-types optional definitely makes sense but in this initial implementation I made a hard dependency for simplicity


[dev-dependencies]
approx = ">= 0.4.0, < 0.6.0"
64 changes: 64 additions & 0 deletions geo-traits/src/coord.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
use geo_types::{Coord, CoordNum, Point};
Copy link
Member

Choose a reason for hiding this comment

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

🧑‍🚒 hot take: It was a mistake to have geo_types::Coordinate separate from geo_types::Point they are redundant, and I'm guessing this only happened because of ideas copied over from Java where a "light weight" representation required splitting the two concepts.

Rust doesn't have the overhead of "primitive type" vs "object". Regardless of how many methods or whatever we add to it, geo-types::Coordinate<f64> will always be the same size as [f64; 2].

The cost of our current approach is frequent confusion about whether a method can/should accept a Coordinate, a Point, or both.

Maybe geo-traits could be an opportunity to fix that wart. We could delete CoordTrait, and have only PointTrait. Both geo-types::Coordinate and geo_types::Point would implement PointTrait, allowing any methods defined in terms of PointTrait to accept both, and finally resolving the question "Should this be a Point or a Coord?"

This is a half baked armchair suggestion, so no changes demanded, but think about it!

In fact, this might be a relatively nice way to start showing the value of a trait system proposal - by simplifying some of our existing APIs.

Copy link
Member

Choose a reason for hiding this comment

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

Also, no judgement on the original geo_types::Coordinate vs geo_types::Point decision. It's gotten us pretty far, and it seemed totally reasonable to me at first too, until years of stubbing my toe on it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd be fully supportive of making Coord and Point both implement a shared PointTrait! I admit I do get confused when to use coord vs point.


pub trait CoordTrait: Send + Sync {
type T: CoordNum + Send + Sync;

/// x component of this coord
fn x(&self) -> Self::T;

/// y component of this coord
fn y(&self) -> Self::T;

/// Returns a tuple that contains the x/horizontal & y/vertical component of the coord.
fn x_y(&self) -> (Self::T, Self::T) {
(self.x(), self.y())
}
}

impl<T: CoordNum + Send + Sync> CoordTrait for Point<T> {
type T = T;

fn x(&self) -> T {
self.0.x
}

fn y(&self) -> T {
self.0.y
}
}

impl<T: CoordNum + Send + Sync> CoordTrait for &Point<T> {
type T = T;

fn x(&self) -> T {
self.0.x
}

fn y(&self) -> T {
self.0.y
}
}

impl<T: CoordNum + Send + Sync> CoordTrait for Coord<T> {
type T = T;

fn x(&self) -> T {
self.x
}

fn y(&self) -> T {
self.y
}
}

impl<T: CoordNum + Send + Sync> CoordTrait for &Coord<T> {
type T = T;

fn x(&self) -> T {
self.x
}

fn y(&self) -> T {
self.y
}
}
87 changes: 87 additions & 0 deletions geo-traits/src/geometry.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
use geo_types::{
CoordNum, Geometry, GeometryCollection, LineString, MultiLineString, MultiPoint, MultiPolygon,
Point, Polygon,
};

use super::{
GeometryCollectionTrait, LineStringTrait, MultiLineStringTrait, MultiPointTrait,
MultiPolygonTrait, PointTrait, PolygonTrait,
};

#[allow(clippy::type_complexity)]
pub trait GeometryTrait<'a>: Send + Sync {
type Point: 'a + PointTrait;
type LineString: 'a + LineStringTrait<'a>;
type Polygon: 'a + PolygonTrait<'a>;
type MultiPoint: 'a + MultiPointTrait<'a>;
type MultiLineString: 'a + MultiLineStringTrait<'a>;
type MultiPolygon: 'a + MultiPolygonTrait<'a>;
type GeometryCollection: 'a + GeometryCollectionTrait<'a>;

fn as_type(
&'a self,
) -> GeometryType<
'a,
Self::Point,
Self::LineString,
Self::Polygon,
Self::MultiPoint,
Self::MultiLineString,
Self::MultiPolygon,
Self::GeometryCollection,
>;
}

#[derive(Debug)]
pub enum GeometryType<'a, P, L, Y, MP, ML, MY, GC>
where
P: 'a + PointTrait,
L: 'a + LineStringTrait<'a>,
Y: 'a + PolygonTrait<'a>,
MP: 'a + MultiPointTrait<'a>,
ML: 'a + MultiLineStringTrait<'a>,
MY: 'a + MultiPolygonTrait<'a>,
GC: 'a + GeometryCollectionTrait<'a>,
{
Point(&'a P),
LineString(&'a L),
Polygon(&'a Y),
MultiPoint(&'a MP),
MultiLineString(&'a ML),
MultiPolygon(&'a MY),
GeometryCollection(&'a GC),
Copy link
Member

Choose a reason for hiding this comment

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

Do you have any thoughts on adding the other types offered in geo-types? Namely Triangle, Line, and Rect

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely something to discuss. I didn't implement them yet because I wasn't sure the pros/cons of how hard that might be for some data structures to implement vs the usefulness in adapting for geo algorithms. If a data structure doesn't natively have a Rect type, should it allow a polygon to fill in as a Rect?

}

impl<'a, T: CoordNum + Send + Sync + 'a> GeometryTrait<'a> for Geometry<T> {
type Point = Point<T>;
type LineString = LineString<T>;
type Polygon = Polygon<T>;
type MultiPoint = MultiPoint<T>;
type MultiLineString = MultiLineString<T>;
type MultiPolygon = MultiPolygon<T>;
type GeometryCollection = GeometryCollection<T>;

fn as_type(
&'a self,
) -> GeometryType<
'a,
Point<T>,
LineString<T>,
Polygon<T>,
MultiPoint<T>,
MultiLineString<T>,
MultiPolygon<T>,
GeometryCollection<T>,
> {
match self {
Geometry::Point(p) => GeometryType::Point(p),
Geometry::LineString(p) => GeometryType::LineString(p),
Geometry::Polygon(p) => GeometryType::Polygon(p),
Geometry::MultiPoint(p) => GeometryType::MultiPoint(p),
Geometry::MultiLineString(p) => GeometryType::MultiLineString(p),
Geometry::MultiPolygon(p) => GeometryType::MultiPolygon(p),
Geometry::GeometryCollection(p) => GeometryType::GeometryCollection(p),
_ => todo!(),
}
}
}
53 changes: 53 additions & 0 deletions geo-traits/src/geometry_collection.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use super::GeometryTrait;
use geo_types::{CoordNum, Geometry, GeometryCollection};
use std::iter::Cloned;
use std::slice::Iter;

pub trait GeometryCollectionTrait<'a>: Send + Sync {
type ItemType: 'a + GeometryTrait<'a>;
type Iter: ExactSizeIterator<Item = Self::ItemType>;

/// An iterator over the geometries in this GeometryCollection
fn geometries(&'a self) -> Self::Iter;

/// The number of geometries in this GeometryCollection
fn num_geometries(&'a self) -> usize;

/// Access to a specified geometry in this GeometryCollection
/// Will return None if the provided index is out of bounds
fn geometry(&'a self, i: usize) -> Option<Self::ItemType>;
}

impl<'a, T: CoordNum + Send + Sync + 'a> GeometryCollectionTrait<'a> for GeometryCollection<T> {
type ItemType = Geometry<T>;
type Iter = Cloned<Iter<'a, Self::ItemType>>;

fn geometries(&'a self) -> Self::Iter {
self.0.iter().cloned()
}

fn num_geometries(&'a self) -> usize {
self.0.len()
}

fn geometry(&'a self, i: usize) -> Option<Self::ItemType> {
self.0.get(i).cloned()
}
}

impl<'a, T: CoordNum + Send + Sync + 'a> GeometryCollectionTrait<'a> for &GeometryCollection<T> {
type ItemType = Geometry<T>;
type Iter = Cloned<Iter<'a, Self::ItemType>>;

fn geometries(&'a self) -> Self::Iter {
self.0.iter().cloned()
}

fn num_geometries(&'a self) -> usize {
self.0.len()
}

fn geometry(&'a self, i: usize) -> Option<Self::ItemType> {
self.0.get(i).cloned()
}
}
19 changes: 19 additions & 0 deletions geo-traits/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
pub use coord::CoordTrait;
pub use geometry::{GeometryTrait, GeometryType};
pub use geometry_collection::GeometryCollectionTrait;
pub use line_string::LineStringTrait;
pub use multi_line_string::MultiLineStringTrait;
pub use multi_point::MultiPointTrait;
pub use multi_polygon::MultiPolygonTrait;
pub use point::PointTrait;
pub use polygon::PolygonTrait;

mod coord;
mod geometry;
mod geometry_collection;
mod line_string;
mod multi_line_string;
mod multi_point;
mod multi_polygon;
mod point;
mod polygon;
54 changes: 54 additions & 0 deletions geo-traits/src/line_string.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
use geo_types::{Coord, CoordNum, LineString};

use super::CoordTrait;
use std::iter::Cloned;
use std::slice::Iter;

pub trait LineStringTrait<'a>: Send + Sync {
type ItemType: 'a + CoordTrait;
type Iter: ExactSizeIterator<Item = Self::ItemType>;

/// An iterator over the coords in this LineString
fn coords(&'a self) -> Self::Iter;

/// The number of coords in this LineString
fn num_coords(&'a self) -> usize;

/// Access to a specified point in this LineString
/// Will return None if the provided index is out of bounds
fn coord(&'a self, i: usize) -> Option<Self::ItemType>;
}

impl<'a, T: CoordNum + Send + Sync + 'a> LineStringTrait<'a> for LineString<T> {
type ItemType = Coord<T>;
type Iter = Cloned<Iter<'a, Self::ItemType>>;

fn coords(&'a self) -> Self::Iter {
self.0.iter().cloned()
}

fn num_coords(&self) -> usize {
self.0.len()
}

fn coord(&'a self, i: usize) -> Option<Self::ItemType> {
self.0.get(i).cloned()
}
}

impl<'a, T: CoordNum + Send + Sync + 'a> LineStringTrait<'a> for &LineString<T> {
type ItemType = Coord<T>;
type Iter = Cloned<Iter<'a, Self::ItemType>>;

fn coords(&'a self) -> Self::Iter {
self.0.iter().cloned()
}

fn num_coords(&self) -> usize {
self.0.len()
}

fn coord(&'a self, i: usize) -> Option<Self::ItemType> {
self.0.get(i).cloned()
}
}
53 changes: 53 additions & 0 deletions geo-traits/src/multi_line_string.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use super::line_string::LineStringTrait;
use geo_types::{CoordNum, LineString, MultiLineString};
use std::iter::Cloned;
use std::slice::Iter;

pub trait MultiLineStringTrait<'a>: Send + Sync {
type ItemType: 'a + LineStringTrait<'a>;
type Iter: ExactSizeIterator<Item = Self::ItemType>;

/// An iterator over the LineStrings in this MultiLineString
fn lines(&'a self) -> Self::Iter;

/// The number of lines in this MultiLineString
fn num_lines(&'a self) -> usize;

/// Access to a specified line in this MultiLineString
/// Will return None if the provided index is out of bounds
fn line(&'a self, i: usize) -> Option<Self::ItemType>;
}

impl<'a, T: CoordNum + Send + Sync + 'a> MultiLineStringTrait<'a> for MultiLineString<T> {
type ItemType = LineString<T>;
type Iter = Cloned<Iter<'a, Self::ItemType>>;

fn lines(&'a self) -> Self::Iter {
self.0.iter().cloned()
}

fn num_lines(&'a self) -> usize {
self.0.len()
}

fn line(&'a self, i: usize) -> Option<Self::ItemType> {
self.0.get(i).cloned()
}
}

impl<'a, T: CoordNum + Send + Sync + 'a> MultiLineStringTrait<'a> for &MultiLineString<T> {
type ItemType = LineString<T>;
type Iter = Cloned<Iter<'a, Self::ItemType>>;

fn lines(&'a self) -> Self::Iter {
self.0.iter().cloned()
}

fn num_lines(&'a self) -> usize {
self.0.len()
}

fn line(&'a self, i: usize) -> Option<Self::ItemType> {
self.0.get(i).cloned()
}
}
Loading