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.

Sign up for GitHub

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

WIP Implement area functions on geometry traits #1021

Closed
wants to merge 15 commits into from
Closed

WIP Implement area functions on geometry traits #1021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9f78526
wip area trait
kylebarron Jun 21, 2023
f233571
fix index
kylebarron Jun 21, 2023
2b7f499
Switch to manually constructing Coord
kylebarron Jun 23, 2023
c3656cc
Remove Coord AddAssign SubAssign
kylebarron Jun 23, 2023
fd62519
Maybe progress?
kylebarron Jun 23, 2023
5b267ca
remove global SubAssign bound
kylebarron Jun 23, 2023
5893b97
Fix compilation, remove send + sync
kylebarron Jun 26, 2023
873575f
Fix tests
kylebarron Jun 27, 2023
5d9ddf5
Comment out ci 1.63
kylebarron Jun 27, 2023
0367c8f
Expose signed and unsigned area for a polygon
kylebarron Jun 27, 2023
2ebb574
Fix test
kylebarron Jun 27, 2023
a61b5e2
Add parametrized type T to all trait definitions
kylebarron Jun 27, 2023
8264083
Try to add signed_area_geometry
kylebarron Jun 27, 2023
c3ae4d5
Remove 'a lifetime on non-iterating trait methods
kylebarron Jun 27, 2023
bfd1c6d
Get multi polygon methods to compile
kylebarron Jun 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ jobs:
# giving us about 6 months of coverage.
#
# Minimum supported rust version (MSRV)
- "georust/geo-ci:rust-1.63"
# - "georust/geo-ci:rust-1.63"
# Two most recent releases - we omit older ones for expedient CI
- "georust/geo-ci:rust-1.65"
- "georust/geo-ci:rust-1.66"
Expand Down Expand Up @@ -96,7 +96,7 @@ jobs:
# giving us about 6 months of coverage.
#
# Minimum supported rust version (MSRV)
- "georust/geo-ci:rust-1.63"
# - "georust/geo-ci:rust-1.63"
# Two most recent releases - we omit older ones for expedient CI
- "georust/geo-ci:rust-1.65"
- "georust/geo-ci:rust-1.66"
Expand All @@ -123,7 +123,7 @@ jobs:
# giving us about 6 months of coverage.
#
# Minimum supported rust version (MSRV)
- "georust/geo-ci:rust-1.63"
# - "georust/geo-ci:rust-1.63"
# Two most recent releases - we omit older ones for expedient CI
- "georust/geo-ci:rust-1.65"
- "georust/geo-ci:rust-1.66"
Expand All @@ -149,7 +149,7 @@ jobs:
# giving us about 6 months of coverage.
#
# Minimum supported rust version (MSRV)
- "georust/geo-ci:rust-1.63"
# - "georust/geo-ci:rust-1.63"
# Two most recent releases - we omit older ones for expedient CI
- "georust/geo-ci:rust-1.65"
- "georust/geo-ci:rust-1.66"
Expand Down
28 changes: 14 additions & 14 deletions geo-traits/src/coord.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use geo_types::{Coord, CoordNum, Point};

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

/// x component of this coord
fn x(&self) -> Self::T;
Expand All @@ -15,50 +15,50 @@ pub trait CoordTrait: Send + Sync {
}
}

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

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

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

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

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

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

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

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

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

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

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

fn y(&self) -> T {
fn y(&self) -> Self::T {
self.y
}
}
48 changes: 25 additions & 23 deletions geo-traits/src/geometry.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ use super::{
};

#[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>;
pub trait GeometryTrait<'a> {
type T: CoordNum;
type Point: 'a + PointTrait<T = Self::T>;
type LineString: 'a + LineStringTrait<'a, T = Self::T>;
Comment on lines +12 to +15
Copy link
Member

Choose a reason for hiding this comment

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

This can now be:

type LineString<'a>: 'a + LineStringTrait<'a, T = Self::T>;

Or something like that

Copy link
Member Author

Choose a reason for hiding this comment

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

What's the benefit to adding <'a> onto the type name as well?

Copy link
Member

Choose a reason for hiding this comment

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

You can remove the lifetime from the trait itself, which means any references to the trait don't need to specify that lifetime. Similar to https://github.com/georust/geo/pull/908/files

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh wow that is super nice!

Copy link
Member Author

@kylebarron kylebarron Nov 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It ended up being a lot more work than expected but I implemented that change in geoarrow/geoarrow-rs#253 (I'm incubating and testing out trait impls there); I'll make a PR to the traits branch with those updated changes

(I still had lifetime issues connecting to geos but that's life 🤷‍♂️ )

type Polygon: 'a + PolygonTrait<'a, T = Self::T>;
type MultiPoint: 'a + MultiPointTrait<'a, T = Self::T>;
type MultiLineString: 'a + MultiLineStringTrait<'a, T = Self::T>;
type MultiPolygon: 'a + MultiPolygonTrait<'a, T = Self::T>;
type GeometryCollection: 'a + GeometryCollectionTrait<'a, T = Self::T>;

fn as_type(
&'a self,
Expand All @@ -35,13 +36,13 @@ pub trait GeometryTrait<'a>: Send + Sync {
#[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>,
P: PointTrait,
L: LineStringTrait<'a>,
Y: PolygonTrait<'a>,
MP: MultiPointTrait<'a>,
ML: MultiLineStringTrait<'a>,
MY: MultiPolygonTrait<'a>,
GC: GeometryCollectionTrait<'a>,
{
Point(&'a P),
LineString(&'a L),
Expand All @@ -52,14 +53,15 @@ where
GeometryCollection(&'a GC),
}

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>;
impl<'a, T: CoordNum + 'a> GeometryTrait<'a> for Geometry<T> {
type T = T;
type Point = Point<Self::T>;
type LineString = LineString<Self::T>;
type Polygon = Polygon<Self::T>;
type MultiPoint = MultiPoint<Self::T>;
type MultiLineString = MultiLineString<Self::T>;
type MultiPolygon = MultiPolygon<Self::T>;
type GeometryCollection = GeometryCollection<Self::T>;

fn as_type(
&'a self,
Expand Down
27 changes: 15 additions & 12 deletions geo-traits/src/geometry_collection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,51 +3,54 @@ 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>;
pub trait GeometryCollectionTrait<'a> {
type T: CoordNum;
type ItemType: 'a + GeometryTrait<'a, T = Self::T>;
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;
fn num_geometries(&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>;
fn geometry(&self, i: usize) -> Option<Self::ItemType>;
}

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

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

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

fn geometry(&'a self, i: usize) -> Option<Self::ItemType> {
fn geometry(&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>;
impl<'a, T: CoordNum + 'a> GeometryCollectionTrait<'a> for &GeometryCollection<T> {
type T = T;
type ItemType = Geometry<Self::T>;
type Iter = Cloned<Iter<'a, Self::ItemType>>;

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

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

fn geometry(&'a self, i: usize) -> Option<Self::ItemType> {
fn geometry(&self, i: usize) -> Option<Self::ItemType> {
self.0.get(i).cloned()
}
}
23 changes: 13 additions & 10 deletions geo-traits/src/line_string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,25 @@ use super::CoordTrait;
use std::iter::Cloned;
use std::slice::Iter;

pub trait LineStringTrait<'a>: Send + Sync {
type ItemType: 'a + CoordTrait;
pub trait LineStringTrait<'a> {
type T: CoordNum;
type ItemType: 'a + CoordTrait<T = Self::T>;
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;
fn num_coords(&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>;
fn coord(&self, i: usize) -> Option<Self::ItemType>;
}

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

fn coords(&'a self) -> Self::Iter {
Expand All @@ -31,13 +33,14 @@ impl<'a, T: CoordNum + Send + Sync + 'a> LineStringTrait<'a> for LineString<T> {
self.0.len()
}

fn coord(&'a self, i: usize) -> Option<Self::ItemType> {
fn coord(&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>;
impl<'a, T: CoordNum + 'a> LineStringTrait<'a> for &LineString<T> {
type T = T;
type ItemType = Coord<Self::T>;
type Iter = Cloned<Iter<'a, Self::ItemType>>;

fn coords(&'a self) -> Self::Iter {
Expand All @@ -48,7 +51,7 @@ impl<'a, T: CoordNum + Send + Sync + 'a> LineStringTrait<'a> for &LineString<T>
self.0.len()
}

fn coord(&'a self, i: usize) -> Option<Self::ItemType> {
fn coord(&self, i: usize) -> Option<Self::ItemType> {
self.0.get(i).cloned()
}
}
27 changes: 15 additions & 12 deletions geo-traits/src/multi_line_string.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,51 +3,54 @@ 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>;
pub trait MultiLineStringTrait<'a> {
type T: CoordNum;
type ItemType: 'a + LineStringTrait<'a, T = Self::T>;
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;
fn num_lines(&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>;
fn line(&self, i: usize) -> Option<Self::ItemType>;
}

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

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

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

fn line(&'a self, i: usize) -> Option<Self::ItemType> {
fn line(&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>;
impl<'a, T: CoordNum + 'a> MultiLineStringTrait<'a> for &MultiLineString<T> {
type T = T;
type ItemType = LineString<Self::T>;
type Iter = Cloned<Iter<'a, Self::ItemType>>;

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

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

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