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 geo-traits crate #1157

Merged
merged 40 commits into from
Oct 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
e337d8a
Add geo-traits crate
kylebarron Feb 26, 2024
7f68df2
Update doc
kylebarron Feb 26, 2024
34ed5be
Update geo-traits/Cargo.toml
kylebarron Feb 27, 2024
d57995e
Merge branch 'main' into kyle/geo-traits-crate
kylebarron Oct 10, 2024
7308df3
Merge branch 'kyle/geo-traits-crate' of github.com:kylebarron/geo int…
kylebarron Oct 10, 2024
ee04201
Update docker images
kylebarron Oct 10, 2024
ad48ee1
Remove CoordTrait and add multiple dimensions
kylebarron Oct 10, 2024
4261836
Module docs
kylebarron Oct 10, 2024
449b39f
rename
kylebarron Oct 10, 2024
f54fffc
Add logical Dimension
kylebarron Oct 11, 2024
db08c55
partialeq and hash
kylebarron Oct 11, 2024
7ecdb7e
return impl Iterator
kylebarron Oct 12, 2024
03f6615
rename to min/max
kylebarron Oct 12, 2024
31435e8
Merge branch 'main' into kyle/geo-traits-crate
kylebarron Oct 12, 2024
1d30066
Specialized implementations of GeometryTrait
kylebarron Oct 13, 2024
ab0bf7b
clearer GeometryTrait naming
kylebarron Oct 13, 2024
33c1ef9
Rename ItemType
kylebarron Oct 13, 2024
6363ad8
Unimplemented versions
kylebarron Oct 14, 2024
9421e05
Add LineTrait and TriangleTrait and improve trait docs
kylebarron Oct 14, 2024
9552705
Fix geo-types empty Polygon
kylebarron Oct 14, 2024
3725844
Merge branch 'main' into kyle/geo-traits-crate
kylebarron Oct 14, 2024
7a2effd
Make iterators private
kylebarron Oct 14, 2024
8dcbe50
Rename Dimension -> Dimensions
kylebarron Oct 14, 2024
6889cae
No default x() and y()
kylebarron Oct 14, 2024
de7ca8a
More descriptive point panics
kylebarron Oct 14, 2024
0fb5ab8
Change to DoubleEndedIterator + ExactSizeIterator
kylebarron Oct 15, 2024
58117ce
change dimensions case
kylebarron Oct 15, 2024
6b697dd
Update geo-traits/src/point.rs
kylebarron Oct 16, 2024
2eb3b03
reorder unchecked
kylebarron Oct 16, 2024
1f68cc3
`PointTrait::is_empty`
kylebarron Oct 16, 2024
b1364ef
Merge branch 'main' into kyle/geo-traits-crate
kylebarron Oct 16, 2024
d49873b
Add sentence to is_empty
kylebarron Oct 16, 2024
7c0e90f
fix doc link
kylebarron Oct 16, 2024
09de278
Use array
kylebarron Oct 18, 2024
8418780
Update geo-traits/src/line.rs
kylebarron Oct 18, 2024
4a9b3cc
typo
kylebarron Oct 22, 2024
ba9bf98
Merge branch 'main' into kyle/geo-traits-crate
kylebarron Oct 22, 2024
4764343
Restore CoordTrait, PointTrait yields Option<CoordTrait>
kylebarron Oct 24, 2024
2b77691
Merge branch 'main' into kyle/geo-traits-crate
kylebarron Oct 24, 2024
711dfac
Update geo-traits/src/coord.rs
kylebarron Oct 25, 2024
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
27 changes: 26 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,32 @@ jobs:
# we don't want to test `proj-network` because it only enables the `proj` feature
- run: cargo test --features "use-proj use-serde"

geo_traits:
name: geo-traits
runs-on: ubuntu-latest
if: "!contains(github.event.head_commit.message, '[skip ci]')"
defaults:
run:
working-directory: geo-traits
strategy:
matrix:
container_image:
# We aim to support rust-stable plus (at least) the prior 3 releases,
# giving us about 6 months of coverage.
#
# Minimum supported rust version (MSRV)
- "ghcr.io/georust/geo-ci:proj-9.4.0-rust-1.75"
# Two most recent releases - we omit older ones for expedient CI
- "ghcr.io/georust/geo-ci:proj-9.4.0-rust-1.78"
- "ghcr.io/georust/geo-ci:proj-9.4.0-rust-1.79"
container:
image: ${{ matrix.container_image }}
steps:
- name: Checkout repository
uses: actions/checkout@v2
- run: cargo check --all-targets
- run: cargo test

geo_postgis:
name: geo-postgis
runs-on: ubuntu-latest
Expand Down Expand Up @@ -184,4 +210,3 @@ jobs:
- name: Checkout repository
uses: actions/checkout@v3
- run: RUSTDOCFLAGS="-D warnings" cargo doc --all-features --no-deps

5 changes: 3 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
resolver = "2"
members = [
"geo",
"geo-types",
"geo-bool-ops-benches",
"geo-postgis",
"geo-test-fixtures",
"geo-traits",
"geo-types",
"jts-test-runner",
"geo-bool-ops-benches",
]

[patch.crates-io]
Expand Down
16 changes: 16 additions & 0 deletions geo-traits/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[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"
edition = "2021"

[dependencies]
geo-types = "0.7"

[dev-dependencies]
141 changes: 141 additions & 0 deletions geo-traits/src/coord.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
use std::marker::PhantomData;

use geo_types::{Coord, CoordNum};

use crate::Dimensions;

/// A trait for accessing data from a generic Coord.
///
/// Refer to [geo_types::Coord] for information about semantics and validity.
pub trait CoordTrait {
/// The coordinate type of this geometry
type T: CoordNum;

/// Dimensions of the coordinate tuple
fn dim(&self) -> Dimensions;

/// Access the n'th (0-based) element of the CoordinateTuple.
/// Returns `None` if `n >= DIMENSION`.
/// See also [`nth_unchecked()`](Self::nth_unchecked).
fn nth(&self, n: usize) -> Option<Self::T> {
if n < self.dim().size() {
Some(self.nth_unchecked(n))
} else {
None
}
}

/// 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())
}

/// Access the n'th (0-based) element of the CoordinateTuple.
/// May panic if n >= DIMENSION.
/// See also [`nth()`](Self::nth).
fn nth_unchecked(&self, n: usize) -> Self::T;
Copy link
Member

@frewsxcv frewsxcv Oct 24, 2024

Choose a reason for hiding this comment

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

When I first read this, I thought this would behave similarly to Vec#get_unchecked which is an unsafe method which does not do a bounds check, hence it being marked as unsafe. In fact, it looks like all functions named *_unchecked Rust's standard library are marked as unsafe, which to me implies they skip the bounds check.

My first though was, what if we had a naming convention like this:

fn nth_checked(n) -> Option<T> {}
fn nth(n) -> T {} // Panics
unsafe fn nth_unchecked(n) -> T

But that may psychologically push people to use the panic'ing version.

I don't know what the right answer is, just wanted to call out that I was surprised when implementing CoordTrait that nth_unchecked was not marked with unsafe.

Copy link
Member

Choose a reason for hiding this comment

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

Another option 🤷🏻

fn nth(n) -> Option<T> {}
fn nth_or_panic(n) -> T {} // Panics
unsafe fn nth_unchecked(n) -> T

Copy link
Member

Choose a reason for hiding this comment

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

But we also definitely don't need to have an unsafe method, in fact probably better to leave it out for this initial implementation

Copy link
Member

@michaelkirk michaelkirk Oct 24, 2024

Choose a reason for hiding this comment

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

fn nth(n) -> Option<T> {}
fn nth_or_panic(n) -> T {} // Panics

That's nice. 👏

- coord.nth_unchecked(3);
+ coord.nth(3).unwrap();

We could also just leave out the panic flavor.

Copy link
Member

Choose a reason for hiding this comment

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

Although I didn't find it in the stdlib, apparently there's quite a bit of precedent for _or_panic in 3rd party crates
https://github.com/search?q=or_panic+language%3ARust&type=code

Copy link
Member Author

Choose a reason for hiding this comment

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

Open to suggestions here. I suppose it's worthwhile having a variant that is actually unsafe (e.g. doesn't even do the bounds check), for situations where you know you're in bounds (like the iterator)

}

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

fn nth_unchecked(&self, n: usize) -> Self::T {
match n {
0 => self.x(),
1 => self.y(),
_ => panic!("Coord only supports 2 dimensions"),
}
}

fn dim(&self) -> Dimensions {
Dimensions::Xy
}

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

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

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

fn nth_unchecked(&self, n: usize) -> Self::T {
match n {
0 => self.x(),
1 => self.y(),
_ => panic!("Coord only supports 2 dimensions"),
}
}

fn dim(&self) -> Dimensions {
Dimensions::Xy
}

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

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

impl<T: CoordNum> CoordTrait for (T, T) {
type T = T;

fn nth_unchecked(&self, n: usize) -> Self::T {
match n {
0 => self.x(),
1 => self.y(),
_ => panic!("(T, T) only supports 2 dimensions"),
}
}

fn dim(&self) -> Dimensions {
Dimensions::Xy
}

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

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

/// An empty struct that implements [CoordTrait].
///
/// This can be used as the `CoordType` of the `GeometryTrait` by implementations that don't have a
/// Coord concept
pub struct UnimplementedCoord<T: CoordNum>(PhantomData<T>);

impl<T: CoordNum> CoordTrait for UnimplementedCoord<T> {
type T = T;

fn dim(&self) -> Dimensions {
unimplemented!()
}

fn nth_unchecked(&self, _n: usize) -> Self::T {
unimplemented!()
}

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

fn y(&self) -> Self::T {
unimplemented!()
}
}
33 changes: 33 additions & 0 deletions geo-traits/src/dimension.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/// The logical dimension of the geometry.
///
///
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum Dimensions {
/// A two-dimensional geometry with X and Y values
Xy,

/// A three-dimensional geometry with X, Y, and Z values
Xyz,

/// A three-dimensional geometry with X, Y, and M values
Xym,

/// A four-dimensional geometry with X, Y, Z, and M values
Xyzm,

/// A geometry with unknown logical type. The contained `usize` value represents the number of
/// physical dimensions.
Unknown(usize),
}

impl Dimensions {
/// The physical number of dimensions in this geometry.
pub fn size(&self) -> usize {
match self {
Self::Xy => 2,
Self::Xyz | Self::Xym => 3,
Self::Xyzm => 4,
Self::Unknown(val) => *val,
}
}
}
Loading
Loading