Use JTS geometries in vector package by jpolchlo · Pull Request #2932 · locationtech/geotrellis

jpolchlo · 2019-05-23T19:14:29Z

Overview

We are now favoring direct use of JTS geometries for improved interoperability with other projects. Scala wrapper classes for Geometry have been snuffed. Many submodules of geotrellis.vector have also been removed in favor of direct usage of the corresponding JTS functionality. Extension methods and companion objects have been added to maintain a crisp, candy shell around JTS to keep most interactions from messing up your fingers. Import geotrellis.vector._ to access these niceties; if it is required, import org.locationtech.jts.{geom => jts} to prevent namespace collisions.

Note: In the REPL, geometries will need to be constructed via the duplicate definitions in the JTS object to avoid namespace clashes that appear to be buggy behavior on the part of the REPL (that is, use JTS.Point(0,0) to construct a point at the origin in interactive sessions, but in compiled code, Point(0,0) will suffice). See core concepts documentation in the guide for more details.

Closes #2930
Closes #2931
Closes #2955

Signed-off-by: jpolchlo [email protected]

Checklist

docs/CHANGELOG.rst updated, if necessary
docs guides update, if necessary
New user API has useful Scaladoc strings
Unit tests added for bug-fix or new feature

jpolchlo · 2019-05-28T13:36:25Z

In the interest of furthering a new RFP process for major changes or new features that we're trying out, I'd like to post my plans for this work, in case there is useful feedback. This will call out some of the salient features of the experimental code.

Proposed deletions

Here is the subset of the vector package that will be removed:

vector/src/main/scala/geotrellis/vector/
├── Dimension.scala
├── GeometryCollectionBuilder.scala
├── GeometryCollection.scala
├── affine
│   ├── AffineTransformation.scala
│   ├── GeometryCollectionTransformationMethods.scala
│   ├── LineTransformationMethods.scala
│   ├── MultiLineTransformationMethods.scala
│   ├── MultiPointTransformationMethods.scala
│   ├── MultiPolygonTransformationMethods.scala
│   ├── PointTransformationMethods.scala
│   └── PolygonTransformationMethods.scala
├── convexhull
│   └── ConvexHullMethods.scala
├── densify
│   └── DensifyMethods.scala
├── dissolve
│   └── DissolveMethods.scala
├── prepared
│   ├── PreparedGeometryMethods.scala
│   └── PreparedGeometry.scala
└── simplify
    └── SimplifyMethods.scala

The sub-packages to be removed add little if anything in the way of functionality. We will favor using JTS directly for these features. Obviously, the basic geometry types will be removed; however, I have opted to this point to keep the companion objects to aid in object creation. This does create some type weirdness, though, in some cases requiring importing JTS into a jts alias to prevent confusion.

Proposed Preservations

vector/src/main/scala/geotrellis/vector/
├── Extent.scala
├── Feature.scala
├── GeomFactory.scala
├── package.scala
├── Projected.scala
├── Results.scala
├── RobustPredicates.scala
├── SeqMethods.scala
├── SpatialIndex.scala
├── conf
│   └── JtsConfig.scala
├── interpolation
│   ├── EmpiricalVariogram.scala
│   ├── GeoKriging.scala
│   ├── InterpolationVector.scala
│   ├── Kriging.scala
│   ├── LeastSquaresFittingNuggetProblem.scala
│   ├── LeastSquaresFittingProblem.scala
│   ├── LinearSemivariogram.scala
│   ├── NonLinearSemivariogram.scala
│   ├── OrdinaryKriging.scala
│   ├── Semivariogram.scala
│   ├── SimpleKriging.scala
│   └── UniversalKriging.scala
├── io
│   ├── json
│   │   ├── CrsFormats.scala
│   │   ├── FeatureFormats.scala
│   │   ├── GeoJson.scala
│   │   ├── GeoJsonSupport.scala
│   │   ├── GeometryFormats.scala
│   │   ├── Implicits.scala
│   │   ├── JsonCRS.scala
│   │   ├── JsonFeatureCollectionMap.scala
│   │   ├── JsonFeatureCollection.scala
│   │   └── Style.scala
│   ├── package.scala
│   ├── WKB
│   │   ├── Implicits.scala
│   │   ├── WKB.scala
│   │   └── WKBWriter.scala
│   └── WKT
│       ├── Implicits.scala
│       └── WKT.scala
├── mesh
│   ├── HalfEdge.scala
│   ├── HalfEdgeTable.scala
│   └── IndexedPointSet.scala
├── reproject
│   ├── Implicits.scala
│   └── Reproject.scala
├── summary
│   └── polygonal
│       └── PolygonalSummaryHandler.scala
├── triangulation
│   ├── BoundaryDelaunay.scala
│   ├── DelaunayStitcher.scala
│   ├── DelaunayTriangulationMethods.scala
│   ├── DelaunayTriangulation.scala
│   ├── Implicits.scala
│   ├── QuadricError.scala
│   ├── StitchedDelaunay.scala
│   ├── TriangleMap.scala
│   └── TriangulationPredicates.scala
├── util
│   └── Intersection.scala
└── voronoi
    ├── DelaunayMethods.scala
    ├── Implicits.scala
    ├── package.scala
    ├── VoronoiDiagram.scala
    └── VoronoiMethods.scala

Notable here is the maintenance of the Extent type, which is easier to work with in a scala context (e.g., pattern matching). We will provide implicit conversion from JTS's Envelope type to facilitate usage. We're also keeping a few classes that provide enriched functionality, like SpatialIndex. Additionally, we provide a pureconfig-configured GeometryFactory object. This will be used throughout the system to build compatible geometries. Also note that the WKB and WKT objects in io provide thread-safe operations, and so should be kept. Finally, the Intersection class in the util subpackage will be moved up to the root package, and will be heavily modified to provide a type-enriched intersection facility that will be more useful for pattern matching.

echeipesh · 2019-06-27T14:20:20Z

    def withSRID(srid: Int) = Projected(g, srid)
  }
+
+  @typeclass trait MultiGeometry[G]


This isn't used anywhere but for an import in ProtobufGeom. Do you think this is something important or just left-over at this point?

Yeah, I'd agree that it's mostly vestigial. Not entirely clear what its value was in the first place.

Signed-off-by: jpolchlo <[email protected]>

echeipesh · 2019-07-10T17:27:59Z

@jpolchlo CHANGELOG is the only outstanding thing, it can be [skip ci] tagged

echeipesh · 2019-07-10T17:30:53Z

-  def distanceToInfiniteLine(a: Point, b: Point): Double =
-    CGAlgorithms.distancePointLinePerpendicular(jtsGeom.getCoordinate, a.toCoordinate, b.toCoordinate)
-}
+object Point extends PointConstructors


I think this is actually the last thing we talked about, keeping the geotrellis.vector.jts.Point is the thing that would provide duplicate imports if org.locationtech.jts.geom._ was imported. IIRC our discussion landed on removing these constructors and using only the ones in JTS object.

We discussed this, and I remember settling on "eh, no, maybe not". The footprint of this "simple" change is wide, necessitating either another import (after we went through the trouble to minimize them), or requiring a clumsy prefix for every geometry creation. It's going to be less common for a user to import org.locationtech.jts.geom._, so I made a note in the docs (https://github.com/locationtech/geotrellis/pull/2932/files#diff-38390c0856b2dfe9eff77f20e8a0d9d1R249) to avoid doing so in a careless fashion. The JTS object feels like a kludge to get around a broken thing, not a thing that I would want to use all the time. I would hope that one day the scala REPL would fix its problems and we can just get rid of it.

Ok, thats not what I recall ending on, but I accept this argument now. The docs certainly help. 👍

Signed-off-by: jpolchlo <[email protected]>

echeipesh

LGTM, will squash-merge on green CI.

Signed-off-by: jpolchlo <[email protected]>

pomadchin · 2019-07-10T22:43:18Z

@jpolchlo sweet PR! 🎉

echeipesh added experimental in progress labels May 29, 2019

echeipesh changed the title ~~[EXPERIMENTAL][WIP] Use JTS geometries in vector package~~ Use JTS geometries in vector package May 29, 2019

This was referenced May 30, 2019

Push JTS vector changes into all of GT #2955

Closed

Get Rid of Compiler Warningings in the vector Package #2965

Closed

jpolchlo force-pushed the experiment/jts-vectors branch 3 times, most recently from 976b7d2 to 2577afb Compare June 27, 2019 13:36

echeipesh reviewed Jun 27, 2019

View reviewed changes

jpolchlo force-pushed the experiment/jts-vectors branch from 57a5cc3 to 5a55294 Compare July 9, 2019 18:23

jpolchlo added 19 commits July 10, 2019 10:33

[WIP] Move to JTS geometries in vector package [skip ci]

61fbd08

Signed-off-by: jpolchlo <[email protected]>

[wip][skip ci] Further work on vector refactor

92cb20c

Remove typedIntersection

f0df18f

Signed-off-by: jpolchlo <[email protected]>

Push JTS changes into raster

c9937a1

Signed-off-by: jpolchlo <[email protected]>

Update vectortile package to JTS

d64b5b5

Signed-off-by: jpolchlo <[email protected]>

Update geotools package to JTS

fc7a669

Signed-off-by: jpolchlo <[email protected]>

Update layer package to JTS

e2e185d

Signed-off-by: jpolchlo <[email protected]>

Update store package to JTS

0a6f948

Signed-off-by: jpolchlo <[email protected]>

Update shapefile package to JTS

10d10ce

Signed-off-by: jpolchlo <[email protected]>

Update spark package to JTS

243f846

Signed-off-by: jpolchlo <[email protected]>

Update geomesa package to JTS

9c52c36

Signed-off-by: jpolchlo <[email protected]>

Repair vector tests

654f0ee

Signed-off-by: jpolchlo <[email protected]>

Fixup after rebase

61d2383

Signed-off-by: jpolchlo <[email protected]>

Fixup after rebase

50fbfd4

Signed-off-by: jpolchlo <[email protected]>

Fix a whoopsie with Extent JSON encoding

7e172ba

Signed-off-by: jpolchlo <[email protected]>

Restore dimensionality check [skip ci]

24c1618

Signed-off-by: jpolchlo <[email protected]>

Prefer & over typedIntersection

417d9cd

Signed-off-by: jpolchlo <[email protected]>

Update vector documentation

585bc74

Signed-off-by: jpolchlo <[email protected]>

Repair vector tests

08363c4

Signed-off-by: jpolchlo <[email protected]>

jpolchlo force-pushed the experiment/jts-vectors branch from 0d8efb1 to 08363c4 Compare July 10, 2019 14:39

echeipesh removed experimental in progress labels Jul 10, 2019

echeipesh reviewed Jul 10, 2019

View reviewed changes

jpolchlo added 3 commits July 10, 2019 14:11

Update CHANGELOG [skip ci]

6ced7bb

Signed-off-by: jpolchlo <[email protected]>

Remove raster dependency from vector-testkit

0f9b843

Signed-off-by: jpolchlo <[email protected]>

Remove unused MultiGeometry typeclass

7f6a61f

Signed-off-by: jpolchlo <[email protected]>

echeipesh approved these changes Jul 10, 2019

View reviewed changes

jpolchlo added 2 commits July 10, 2019 15:32

Drop unused imports

ce7ac99

Signed-off-by: jpolchlo <[email protected]>

Drop unused import

9e42206

Signed-off-by: jpolchlo <[email protected]>

echeipesh merged commit 750d599 into locationtech:master Jul 11, 2019

jpolchlo mentioned this pull request Aug 8, 2019

Make sure multi-geometry constructors are available in the shell #3046

Merged

Navigation Menu

Repository navigation

Use JTS geometries in vector package - #2932

Use JTS geometries in vector package#2932
echeipesh merged 24 commits into
locationtech:masterfrom
jpolchlo:experiment/jts-vectors

jpolchlo commented May 23, 2019 •

edited by echeipesh

Loading

Uh oh!

jpolchlo commented May 28, 2019

Uh oh!

echeipesh Jun 27, 2019

Uh oh!

jpolchlo Jun 27, 2019

Uh oh!

echeipesh commented Jul 10, 2019

Uh oh!

echeipesh Jul 10, 2019

Uh oh!

jpolchlo Jul 10, 2019

Uh oh!

echeipesh Jul 10, 2019

Uh oh!

echeipesh left a comment

Uh oh!

pomadchin commented Jul 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jpolchlo commented May 23, 2019 • edited by echeipesh Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Checklist

Uh oh!

jpolchlo commented May 28, 2019

Proposed deletions

Proposed Preservations

Uh oh!

echeipesh Jun 27, 2019

Choose a reason for hiding this comment

Uh oh!

jpolchlo Jun 27, 2019

Choose a reason for hiding this comment

Uh oh!

echeipesh commented Jul 10, 2019

Uh oh!

echeipesh Jul 10, 2019

Choose a reason for hiding this comment

Uh oh!

jpolchlo Jul 10, 2019

Choose a reason for hiding this comment

Uh oh!

echeipesh Jul 10, 2019

Choose a reason for hiding this comment

Uh oh!

echeipesh left a comment

Choose a reason for hiding this comment

Uh oh!

pomadchin commented Jul 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jpolchlo commented May 23, 2019 •

edited by echeipesh

Loading