Skip to content
This repository has been archived by the owner on Apr 2, 2022. It is now read-only.

JTS TestRunner which supports most Centroid and ConvexHull cases. #1

Merged
merged 2 commits into from
Feb 25, 2021

Conversation

michaelkirk
Copy link
Member

@michaelkirk michaelkirk commented Feb 20, 2021

Centroid currently fails some test against master, but the ones that run pass based on the current branch patch (being upstreamed here georust/geo#629).

We still skip a couple of Centroid tests because we don't yet have a way to handle GEOMETRYCOLLECTION(POINT EMPTY), see: georust/wkt#61

Some ConvexHull tests are also failing, but I haven't looked into these failures at all, except to notice that at least some of the failures are because, whereas geo always returns a convex hull polygon, JTS actually will return other types.

/cc @rmanoka

@michaelkirk
Copy link
Member Author

Per your advice I've put this in a separate crate so as not to pull in MB of test data into the geo repo. If you feel good about the approach, I'd be happy to move it to georust and set up CI and whatnot.

My hope is that ultimately we'll have geo's CI run these tests.

Centroid currently fails some test against master, but I will address
this in an upcoming PR to geo.

Plus we skip a couple of tests because we don't yet have a way to handle
collections of "POINT EMPTY".

The ConvexHull cases currently fail, at least some of which is because
geo always returns a Polygon for ConvexHull, whereas JTS might return
other geometry types

#[test]
// several of the ConvexHull tests are currently failing
#[ignore]
Copy link
Member Author

Choose a reason for hiding this comment

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

run this ignored test to see the failing convex hull cases.

@rmanoka
Copy link
Contributor

rmanoka commented Feb 20, 2021

Very nice work @michaelkirk ; thanks! I'll check out the convex hull and try to understand what's going on. It would indeed be good to move this to georust and link to geo CI.

@michaelkirk michaelkirk mentioned this pull request Feb 20, 2021
8 tasks
@michaelkirk
Copy link
Member Author

I'm going to merge this and update my Centroid PR to reference this.

I'm also going to add a README with general instructions on how to add support for new operations.

@michaelkirk michaelkirk merged commit 9ddbe8b into master Feb 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants