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

Jump to bottom

Store a single geometry inside Wkt #72

Merged
merged 3 commits into from
Jan 20, 2022
Merged

Store a single geometry inside Wkt #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e2de3b
Store a single geometry inside Wkt
lnicola Jan 19, 2022
f601f85
Fix some warnings
lnicola Jan 19, 2022
cf69d2a
Update changelog
lnicola Jan 20, 2022
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
6 changes: 5 additions & 1 deletion CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

## Unreleased
### Changed
* Now accepts `MULTIPOINT`s with less parentheses, as output by `ST_AsText` in postgis:
* Now accepts `MULTIPOINT`s with fewer parentheses, as output by `ST_AsText` in postgis:
`MULTIPOINT(0 1, 2 3)` in addition to `MULTIPOINT((0 1), (2 3))`
* BREAKING: Replace `Wkt::items` with `Wkt::item` and remove `Wkt::add_item()`.
* <https://github.com/georust/wkt/pull/72>
* BREAKING: Reject empty strings instead of parsing them into an empty `Wkt`.
* <https://github.com/georust/wkt/pull/72>

## 0.9.2 - 2020-04-30
### Added
Expand Down
2 changes: 0 additions & 2 deletions benches/parse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
extern crate criterion;
extern crate wkt;

use criterion::Criterion;

fn criterion_benchmark(c: &mut criterion::Criterion) {
c.bench_function("parse small", |bencher| {
let s = include_str!("./small.wkt");
Expand Down
74 changes: 15 additions & 59 deletions src/conversion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,41 +42,31 @@ where
{
type Error = Error;

fn try_from(mut wkt: Wkt<T>) -> Result<Self, Self::Error> {
if wkt.items.len() == 1 {
Self::try_from(wkt.items.pop().unwrap())
} else {
Geometry::GeometryCollection(GeometryCollection(wkt.items)).try_into()
}
fn try_from(wkt: Wkt<T>) -> Result<Self, Self::Error> {
Self::try_from(wkt.item)
}
}

#[macro_use]
macro_rules! try_from_wkt_impl {
($($type: ident),+) => {
$(
/// Convert a Wkt enum into a specific geo-type
impl<T: CoordFloat> TryFrom<Wkt<T>> for geo_types::$type<T> {
type Error = Error;

fn try_from(mut wkt: Wkt<T>) -> Result<Self, Self::Error> {
match wkt.items.len() {
1 => {
let item = wkt.items.pop().unwrap();
let geometry = geo_types::Geometry::try_from(item)?;
Self::try_from(geometry).map_err(|e| {
match e {
geo_types::Error::MismatchedGeometry { expected, found } => {
Error::MismatchedGeometry { expected, found }
}
// currently only one error type in geo-types error enum, but that seems likely to change
#[allow(unreachable_patterns)]
other => Error::External(Box::new(other)),
}
})
fn try_from(wkt: Wkt<T>) -> Result<Self, Self::Error> {
let item = wkt.item;
let geometry = geo_types::Geometry::try_from(item)?;
Self::try_from(geometry).map_err(|e| {
match e {
geo_types::Error::MismatchedGeometry { expected, found } => {
Error::MismatchedGeometry { expected, found }
}
// currently only one error type in geo-types error enum, but that seems likely to change
#[allow(unreachable_patterns)]
other => Error::External(Box::new(other)),
}
other => Err(Error::WrongNumberOfGeometries(other)),
}
})
}
}
)+
Expand Down Expand Up @@ -321,48 +311,14 @@ mod tests {
m: None,
}))
.as_item();
let mut wkt = Wkt::new();
wkt.add_item(w_point);
let wkt = Wkt { item: w_point };

let converted = geo_types::Geometry::try_from(wkt).unwrap();
let g_point: geo_types::Point<f64> = geo_types::Point::new(1.0, 2.0);

assert_eq!(converted, geo_types::Geometry::Point(g_point));
}

#[test]
fn convert_collection_wkt() {
let w_point_1 = Point(Some(Coord {
x: 1.0,
y: 2.0,
z: None,
m: None,
}))
.as_item();
let w_point_2 = Point(Some(Coord {
x: 3.0,
y: 4.0,
z: None,
m: None,
}))
.as_item();
let mut wkt = Wkt::new();
wkt.add_item(w_point_1);
wkt.add_item(w_point_2);

let converted = geo_types::Geometry::try_from(wkt).unwrap();
let geo_collection: geo_types::GeometryCollection<f64> =
geo_types::GeometryCollection(vec![
geo_types::Point::new(1.0, 2.0).into(),
geo_types::Point::new(3.0, 4.0).into(),
]);

assert_eq!(
converted,
geo_types::Geometry::GeometryCollection(geo_collection)
);
}

#[test]
fn convert_empty_point() {
let point = Point(None);
Expand Down
12 changes: 3 additions & 9 deletions src/deserialize.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,8 @@ where
where
E: Error,
{
let mut wkt = Wkt::from_str(s).map_err(|e| serde::de::Error::custom(e))?;
if wkt.items.len() == 1 {
Ok(wkt.items.remove(0))
} else {
Err(serde::de::Error::custom(
"WKT should have only 1 Geometry item",
))
}
let wkt = Wkt::from_str(s).map_err(|e| serde::de::Error::custom(e))?;
Ok(wkt.item)
}
}

Expand Down Expand Up @@ -197,7 +191,7 @@ mod tests {
.deserialize_any(WktVisitor::<f64>::default())
.unwrap();
assert!(matches!(
wkt.items[0],
wkt.item,
Geometry::Point(Point(Some(Coord {
x: _, // floating-point types cannot be used in patterns
y: _, // floating-point types cannot be used in patterns
Expand Down
40 changes: 12 additions & 28 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,28 +131,19 @@ pub struct Wkt<T>
where
T: WktFloat,
{
pub items: Vec<Geometry<T>>,
pub item: Geometry<T>,
Copy link
Member Author

Choose a reason for hiding this comment

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

Wkt<T>(Geometry<T>) might be better here? Not sure.

Copy link
Member

Choose a reason for hiding this comment

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

No strong opinion from me, but I have a slight preference for how you have 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 think I'd prefer renaming Geometry to Wkt or an into_geometry() method.

Copy link
Member

Choose a reason for hiding this comment

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

No strong opinion from me, but I have a slight preference for how you have it.

Same, but do what you feel is best here. Where does the into_geometry() method come into 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.

Where does the into_geometry() method come into it?

I meant Wkt::into_geometry(self) -> Geometry.

I'd say keep item for now and maybe change it in a follow-up PR if we figure out a better way.

}

impl<T> Wkt<T>
where
T: WktFloat + FromStr + Default,
{
pub fn new() -> Self {
Wkt { items: vec![] }
}

pub fn add_item(&mut self, item: Geometry<T>) {
self.items.push(item);
}

pub fn from_str(wkt_str: &str) -> Result<Self, &'static str> {
let tokens = Tokens::from_str(wkt_str);
Wkt::from_tokens(tokens)
}

fn from_tokens(tokens: Tokens<T>) -> Result<Self, &'static str> {
let mut wkt = Wkt::new();
let mut tokens = tokens.peekable();
let word = match tokens.next() {
Some(Token::Word(word)) => {
Expand All @@ -161,14 +152,12 @@ where
}
word
}
None => return Ok(wkt),
_ => return Err("Invalid WKT format"),
};
match Geometry::from_word_and_tokens(&word, &mut tokens) {
Ok(item) => wkt.add_item(item),
Ok(item) => Ok(Wkt { item }),
Err(s) => return Err(s),
}
Ok(wkt)
}
}

Expand Down Expand Up @@ -230,32 +219,29 @@ mod tests {

#[test]
fn empty_string() {
let wkt: Wkt<f64> = Wkt::from_str("").ok().unwrap();
assert_eq!(0, wkt.items.len());
let res: Result<Wkt<f64>, _> = Wkt::from_str("");
assert!(res.is_err());
}

#[test]
fn empty_items() {
let mut wkt: Wkt<f64> = Wkt::from_str("POINT EMPTY").ok().unwrap();
assert_eq!(1, wkt.items.len());
match wkt.items.pop().unwrap() {
let wkt: Wkt<f64> = Wkt::from_str("POINT EMPTY").ok().unwrap();
match wkt.item {
Geometry::Point(Point(None)) => (),
_ => unreachable!(),
};

let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOLYGON EMPTY").ok().unwrap();
assert_eq!(1, wkt.items.len());
match wkt.items.pop().unwrap() {
let wkt: Wkt<f64> = Wkt::from_str("MULTIPOLYGON EMPTY").ok().unwrap();
match wkt.item {
Geometry::MultiPolygon(MultiPolygon(polygons)) => assert_eq!(polygons.len(), 0),
_ => unreachable!(),
};
}

#[test]
fn lowercase_point() {
let mut wkt: Wkt<f64> = Wkt::from_str("point EMPTY").ok().unwrap();
assert_eq!(1, wkt.items.len());
match wkt.items.pop().unwrap() {
let wkt: Wkt<f64> = Wkt::from_str("point EMPTY").ok().unwrap();
match wkt.item {
Geometry::Point(Point(None)) => (),
_ => unreachable!(),
};
Expand All @@ -272,10 +258,8 @@ mod tests {

#[test]
fn support_jts_linearring() {
let mut wkt: Wkt<f64> = Wkt::from_str("linearring (10 20, 30 40)").ok().unwrap();
assert_eq!(1, wkt.items.len());

match wkt.items.pop().unwrap() {
let wkt: Wkt<f64> = Wkt::from_str("linearring (10 20, 30 40)").ok().unwrap();
match wkt.item {
Geometry::LineString(_ls) => (),
_ => panic!("expected to be parsed as a LINESTRING"),
};
Expand Down
4 changes: 1 addition & 3 deletions src/towkt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,6 @@ where
{
fn to_wkt(&self) -> Wkt<T> {
let w_geom = g_geom_to_w_geom(&self);
Wkt {
items: vec![w_geom],
}
Wkt { item: w_geom }
}
}
15 changes: 6 additions & 9 deletions src/types/geometrycollection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,10 @@ mod tests {

#[test]
fn basic_geometrycollection() {
let mut wkt: Wkt<f64> = Wkt::from_str("GEOMETRYCOLLECTION (POINT (8 4)))")
let wkt: Wkt<f64> = Wkt::from_str("GEOMETRYCOLLECTION (POINT (8 4)))")
.ok()
.unwrap();
assert_eq!(1, wkt.items.len());
let items = match wkt.items.pop().unwrap() {
let items = match wkt.item {
Geometry::GeometryCollection(GeometryCollection(items)) => items,
_ => unreachable!(),
};
Expand All @@ -101,12 +100,10 @@ mod tests {

#[test]
fn complex_geometrycollection() {
let mut wkt: Wkt<f64> =
Wkt::from_str("GEOMETRYCOLLECTION (POINT (8 4),LINESTRING(4 6,7 10)))")
.ok()
.unwrap();
assert_eq!(1, wkt.items.len());
let items = match wkt.items.pop().unwrap() {
let wkt: Wkt<f64> = Wkt::from_str("GEOMETRYCOLLECTION (POINT (8 4),LINESTRING(4 6,7 10)))")
.ok()
.unwrap();
let items = match wkt.item {
Geometry::GeometryCollection(GeometryCollection(items)) => items,
_ => unreachable!(),
};
Expand Down
5 changes: 2 additions & 3 deletions src/types/linestring.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,8 @@ mod tests {

#[test]
fn basic_linestring() {
let mut wkt = Wkt::from_str("LINESTRING (10 -20, -0 -0.5)").ok().unwrap();
assert_eq!(1, wkt.items.len());
let coords = match wkt.items.pop().unwrap() {
let wkt = Wkt::from_str("LINESTRING (10 -20, -0 -0.5)").ok().unwrap();
let coords = match wkt.item {
Geometry::LineString(LineString(coords)) => coords,
_ => unreachable!(),
};
Expand Down
5 changes: 2 additions & 3 deletions src/types/multilinestring.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,10 @@ mod tests {

#[test]
fn basic_multilinestring() {
let mut wkt: Wkt<f64> = Wkt::from_str("MULTILINESTRING ((8 4, -3 0), (4 0, 6 -10))")
let wkt: Wkt<f64> = Wkt::from_str("MULTILINESTRING ((8 4, -3 0), (4 0, 6 -10))")
.ok()
.unwrap();
assert_eq!(1, wkt.items.len());
let lines = match wkt.items.pop().unwrap() {
let lines = match wkt.item {
Geometry::MultiLineString(MultiLineString(lines)) => lines,
_ => unreachable!(),
};
Expand Down
20 changes: 8 additions & 12 deletions src/types/multipoint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,8 @@ mod tests {

#[test]
fn basic_multipoint() {
let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT ((8 4), (4 0))").ok().unwrap();
assert_eq!(1, wkt.items.len());
let points = match wkt.items.pop().unwrap() {
let wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT ((8 4), (4 0))").ok().unwrap();
let points = match wkt.item {
Geometry::MultiPoint(MultiPoint(points)) => points,
_ => unreachable!(),
};
Expand All @@ -83,9 +82,8 @@ mod tests {

#[test]
fn postgis_style_multipoint() {
let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT (8 4, 4 0)").unwrap();
assert_eq!(1, wkt.items.len());
let points = match wkt.items.pop().unwrap() {
let wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT (8 4, 4 0)").unwrap();
let points = match wkt.item {
Geometry::MultiPoint(MultiPoint(points)) => points,
_ => unreachable!(),
};
Expand All @@ -94,9 +92,8 @@ mod tests {

#[test]
fn mixed_parens_multipoint() {
let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT (8 4, (4 0))").unwrap();
assert_eq!(1, wkt.items.len());
let points = match wkt.items.pop().unwrap() {
let wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT (8 4, (4 0))").unwrap();
let points = match wkt.item {
Geometry::MultiPoint(MultiPoint(points)) => points,
_ => unreachable!(),
};
Expand All @@ -105,9 +102,8 @@ mod tests {

#[test]
fn empty_multipoint() {
let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT EMPTY").unwrap();
assert_eq!(1, wkt.items.len());
let points = match wkt.items.pop().unwrap() {
let wkt: Wkt<f64> = Wkt::from_str("MULTIPOINT EMPTY").unwrap();
let points = match wkt.item {
Geometry::MultiPoint(MultiPoint(points)) => points,
_ => unreachable!(),
};
Expand Down
5 changes: 2 additions & 3 deletions src/types/multipolygon.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,10 @@ mod tests {

#[test]
fn basic_multipolygon() {
let mut wkt: Wkt<f64> = Wkt::from_str("MULTIPOLYGON (((8 4)), ((4 0)))")
let wkt: Wkt<f64> = Wkt::from_str("MULTIPOLYGON (((8 4)), ((4 0)))")
.ok()
.unwrap();
assert_eq!(1, wkt.items.len());
let polygons = match wkt.items.pop().unwrap() {
let polygons = match wkt.item {
Geometry::MultiPolygon(MultiPolygon(polygons)) => polygons,
_ => unreachable!(),
};
Expand Down
Loading