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 Support for GeoTools SimpleFeature #1495

Merged
merged 17 commits into from Jul 8, 2016

Conversation

Projects
None yet
4 participants
@jamesmcclain
Member

jamesmcclain commented May 23, 2016

In this pull request, support is added for converting GeoTools SimpleFeatures into Geotrellis Feature objects and vice-versa.

Still Needs

  • Improved API
    • toSimpleFeature (Geometry)
    • toSimpleFeature (Feature)
    • toGeometry
    • toFeature
  • Depends on #1502
  • Suitable shapefiles for tests
  • More tests

@jamesmcclain jamesmcclain force-pushed the jamesmcclain:feature/jwm/SimpleFeature branch 2 times, most recently from e0ad08c to fe68ed3 May 24, 2016

@jamesmcclain jamesmcclain changed the title from [WiP] Add Support for GeoTools SimpleFeature to Add Support for GeoTools SimpleFeature May 24, 2016

"org.geotools" % "gt-coverage" % Version.geotools,
"org.geotools" % "gt-geotiff" % Version.geotools,
"org.geotools" % "gt-epsg-hsql" % Version.geotools,
"org.apache.spark" %% "spark-core" % Version.spark % "provided",

This comment has been minimized.

@echeipesh

echeipesh May 25, 2016

Contributor

This doesn't seem to be used.

This comment has been minimized.

@jamesmcclain

jamesmcclain May 25, 2016

Member

Indeed, the first few commits of this PR were taken from my other GeoTools-related PR. When the GridCoverage2D stuff that Rob is working on goes in, I will rebase and this will go away.

@jamesmcclain jamesmcclain force-pushed the jamesmcclain:feature/jwm/SimpleFeature branch from fe68ed3 to 4aa2876 May 26, 2016

@jamesmcclain jamesmcclain force-pushed the jamesmcclain:feature/jwm/SimpleFeature branch 4 times, most recently from 7b28582 to 1746e54 Jun 6, 2016

@lossyrob

This comment has been minimized.

Member

lossyrob commented Jun 29, 2016

This API should be refactored to follow the ToGridCoverage2DMethods design pattern, to provide this functionality as implicit methods, and to provide better usability.

Client side view of the API should be e.g.

val point: Point = ???
val crs: CRS = ???
val data: Map[(String, Any)] = ???

point.toSimpleFeature()
point.toSimpleFeature(crs)
point.toSimpleFeature(crs, data)
point.toSimpleFeature(data)

val simpleFeature: SimpleFeature = ???
val point: Point = simpleFeature.toGeometry[Point]
val geom: Geometry = simpleFeature.toGeometry[Geometry]
val pointFeature: PointFeature[Map[String, Object]] = simpleFeature.toFeature[Point]

// this opens the door for an implicit that converts the map to a case class, e.g.
case class Foo(x: Int, y: String)
implicit def mapToFoo(map: Map[String, Object]): Foo = ???

val pointFeature: PointFeature[Foo] = simpleFeature.toFeature[Point, Foo](mapToFoo) // mapToFoo implicit param
@jamesmcclain

This comment has been minimized.

Member

jamesmcclain commented Jun 29, 2016

Okay, I will see if I can put those changes in soon.

@jamesmcclain jamesmcclain force-pushed the jamesmcclain:feature/jwm/SimpleFeature branch from 1746e54 to a4cc264 Jul 1, 2016

@jamesmcclain

This comment has been minimized.

Member

jamesmcclain commented Jul 5, 2016

I believe that all comments prior to this one have been addressed.

}
}
def apply(simpleFeature: SimpleFeature): Feature[Geometry, immutable.Map[String, Object]] = {

This comment has been minimized.

@lossyrob

lossyrob Jul 5, 2016

Member

We should have this typed on [G <: Geometry: ClassTag] then use a combination of

https://github.com/geotrellis/geotrellis/blob/master/vector/src/main/scala/geotrellis/vector/Geometry.scala#L81

and

https://github.com/geotrellis/geotrellis/blob/master/vector/src/main/scala/geotrellis/vector/Geometry.scala#L105

To return the correct type.

Then the asInstanceOf cast below gets pushed down into the moment we translate from a JTS geometry.

We could call SimpleFeatureToFeature[Geometry] to keep the most generic type

@@ -20,4 +23,11 @@ package object geotools {
implicit class withGridCoverage2DConversionMethods(val self: GridCoverage2D) extends MethodExtensions[GridCoverage2D]
with GridCoverage2DConversionMethods
implicit class withGeometryToSimpleFeatureMethods[G <: Geometry](val self: G) extends MethodExtensions[G]
with GeometryToSimpleFeatureMethods[G]

This comment has been minimized.

@lossyrob

lossyrob Jul 5, 2016

Member

Need methods for Feature[G, D] => SimpleFeature

@jamesmcclain

This comment has been minimized.

Member

jamesmcclain commented Jul 5, 2016

I believe that all comments prior to this one have been addressed.

}
}
def apply[G <: Geometry : ClassTag](simpleFeature: SimpleFeature): Feature[G, immutable.Map[String, Object]] = {

This comment has been minimized.

@moradology

moradology Jul 7, 2016

Contributor

I wonder about our ability to pull off a roundtrip without an inverse of the transmute function defined for the Feature -> SimpleFeature translation

@jamesmcclain jamesmcclain force-pushed the jamesmcclain:feature/jwm/SimpleFeature branch from 519c7a4 to 6868341 Jul 8, 2016

@jamesmcclain

This comment has been minimized.

Member

jamesmcclain commented Jul 8, 2016

Map[String, Object] has been changed to Map[String, AnyRef]

@lossyrob

This comment has been minimized.

Member

lossyrob commented Jul 8, 2016

+1

@lossyrob lossyrob merged commit 374e6c1 into locationtech:master Jul 8, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jamesmcclain jamesmcclain deleted the jamesmcclain:feature/jwm/SimpleFeature branch Jul 8, 2016

@lossyrob lossyrob added this to the 1.0 milestone Oct 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment