Switch to scala 2.12 as build default #3132

echeipesh · 2019-10-21T16:39:36Z

Switching to scala 2.12 gives us 25% faster build time on this project and better compatibility with tools like metals. Developer time is precious so that's nothing to sneeze at.

There are couple of issues with this PR that are still outstanding

geowave version we're using is 0.9.3 and has a strict dependency on scala 2.11 dependencies
geomesa version has similar problem
publish scripts need to be adjusted
deprecation warnings need to be resolved

Overall the geowave and geomesa projects are being unlinked from the main build for a bit as those problems look like they'll be harder to solve and both of those projects are experimental and not in heavy use.

CQs required:

Fixes #3139

project/Dependencies.scala

.travis/build-and-test-set-1.sh

pomadchin · 2019-10-23T12:45:17Z

The build fails because of this lightbend-labs/mima#422 the workaround is

resolvers += Resolver.url(
  "typesafe sbt-plugins",
  url("https://dl.bintray.com/typesafe/sbt-plugins")
)(Resolver.ivyStylePatterns)

- geomesa does not cross publish for 2.12 - geowave does not corss publish for 2.12 These projects are both experimental so they're bing unlinked from the build until we either update or figure out the best way to deal with them.

Two cases here: - Type constraint was added for clarity but is actual constraint on the match - We know the numerical domain of the match in internal API

move commonSettings to Settings Use Def.settings

pomadchin · 2019-11-21T18:55:12Z

I requested a PMC approval, but not sure would it be completed successfully or not, due to ongoing changes in the locationtech eclipse structure.

pomadchin

LGTM, added a couple of comments to discuss, but it is good to go as it is. Also all CQs were approved by Jim and awaiting the IP team review. Merging once all CQs are closed!

pomadchin · 2019-11-22T13:38:07Z

project/Dependencies.scala

+  }
+
+  def monocle(module: String) = Def.setting {
+    "com.github.julien-truffaut" %% s"monocle-$module" % ver("1.5.1-cats", "2.0.0").value


I think RasterFrames are using the syntax like this?

That's where I lifted this approach but I've certainly seen it in other projects. What do you think? I'm in favor of collapsing the version numbers to here so there is a minimum amount of indirection in the build overall.

pomadchin · 2019-11-22T13:40:04Z

.travis/build-and-test-set-1.sh

@@ -10,9 +10,7 @@
  "project vectortile" test \
  "project util" test \
  "project gdal" test \
-  "project geowave" compile test:compile \


I hope we won't forget to remove the code from the codebase.

pomadchin · 2019-11-22T13:40:58Z

doc-examples/src/main/scala/geotrellis/doc/examples/spark/EuclideanDistanceExamples.scala

@@ -58,7 +58,7 @@ object EuclideanDistanceExamples {
    val tileRDD: RDD[(SpatialKey, Tile)] = inputRDD.euclideanDistance(ld)

    val maxDistance = tileRDD.map(_._2.findMinMaxDouble).collect.foldLeft(-1.0/0.0){ (max, minMax) => scala.math.max(max, minMax._2) }
-    val cm = ColorMap((0.0 to maxDistance by (maxDistance/512)).toArray, ColorRamps.BlueToRed)
+    val cm = ColorMap(Range.BigDecimal.inclusive(0.0, maxDistance, maxDistance/512).map(_.toDouble).toArray, ColorRamps.BlueToRed)


What a syntax!

Oy ... I know. I get what they're saying with this deprecation but this is the least-worst alternative. I want to keep the number of warnings down so we don't go into auto-ignore warning mode and miss something that matters though.

pomadchin · 2019-11-22T13:41:50Z

doc-examples/src/main/scala/geotrellis/doc/examples/spark/VoxelKey.scala

@@ -66,6 +65,9 @@ object VoxelKey {
      (k, sk) => VoxelKey(sk.col, sk.row, k.z)
    )
  }
+
+  implicit val voxelKeyEncoder: Encoder[VoxelKey] = deriveEncoder[VoxelKey]


Hm, would the annotation here more user friendly? (less lines ~ easier to understand)

They didn't compile for some reason and this was the work-around. I didn't look into why they didn't because it seems like the annotation has been very fragile in other cases and using deriveEncoder is mostly the default in my mind.

scripts/buildall.sh

pomadchin · 2019-11-22T13:46:26Z

spark/src/main/scala/geotrellis/spark/matching/RDDHistogramMatching.scala

@@ -94,7 +94,7 @@ object RDDHistogramMatching {
    sourceHistograms: Seq[Histogram[T1]],
    targetHistograms: Seq[Histogram[T2]]
  ): RDD[(K, MultibandTile)] =
-    rdd.map({ case (key, tile: MultibandTile) =>
+    rdd.map({ case (key, tile) =>


You don't need extra brackets here: rdd.map { case (key, tile) => (key, HistogramMatching(tile, sourceHistograms, targetHistograms)) }. This comment can be applied to all changes below btw.

I'm going for minimum code change outside of configuration here, would rather not get sucked into format shuffling. Having said that once we get the guild and publishing sorted out we really should look into enforcing a code formatting standard for the library.

pomadchin · 2019-11-22T13:48:39Z

version.sbt

@@ -0,0 +1 @@
+ThisBuild / version := "3.1.1-SNAPSHOT"


Is keeping the version in a separate file more convenient? With my gt-contrib and gt-server experiences I didn't notice any real wins ):

Opening the discussion with this. Felt like I should pull this out of Version.scala which always stumps me. I'd be also OK with putting this version into build.sbt with other build settings.

Let’s try with a version in a file; but I’m just worrying that it would be just an extra file and the story would be the same as a Versions class in a separate file.

echeipesh added in progress build labels Oct 21, 2019

echeipesh commented Oct 21, 2019

View reviewed changes

project/Dependencies.scala Show resolved Hide resolved

pomadchin reviewed Oct 22, 2019

View reviewed changes

.travis/build-and-test-set-1.sh Outdated Show resolved Hide resolved

notthatbreezy mentioned this pull request Oct 23, 2019

Upgrade Cats Ecosystem Dependencies #3139

Closed

echeipesh force-pushed the build/212-default branch from 2a1cf9f to db18201 Compare October 30, 2019 16:36

pomadchin added the needs cq label Oct 30, 2019

echeipesh self-assigned this Oct 30, 2019

echeipesh and others added 8 commits November 21, 2019 13:42

Switch default project build to scala 2.12

c510a7d

- geomesa does not cross publish for 2.12 - geowave does not corss publish for 2.12 These projects are both experimental so they're bing unlinked from the build until we either update or figure out the best way to deal with them.

Fix not exhaustive match exceptions

f31c920

Two cases here: - Type constraint was added for clarity but is actual constraint on the match - We know the numerical domain of the match in internal API

Remove geowave/geomesa from build scripts

b3e661d

Fix more deprecation warnings

5b88b8d

Use Range.BigDecimal

de1babb

Bump 2.12 deps up

c910222

move commonSettings to Settings Use Def.settings

Add resolver for mima-plugin

fee24aa

Fix: VoxelKey encoder in the docs

9ba482b

echeipesh force-pushed the build/212-default branch from 6ccc9af to 9ba482b Compare November 21, 2019 18:43

echeipesh requested a review from pomadchin November 21, 2019 18:44

echeipesh assigned pomadchin Nov 21, 2019

pomadchin approved these changes Nov 22, 2019

View reviewed changes

pomadchin removed the in progress label Nov 22, 2019

remove comment from build

828ab1c

pomadchin merged commit 1d1561c into locationtech:master Nov 26, 2019

echeipesh mentioned this pull request Dec 18, 2019

GeoTrellis 3.2.0 geotrellis/maml#112

Merged

1 task

pomadchin removed the needs cq label Feb 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to scala 2.12 as build default #3132

Switch to scala 2.12 as build default #3132

echeipesh commented Oct 21, 2019 •

edited

Loading

pomadchin commented Oct 23, 2019 •

edited

Loading

pomadchin commented Nov 21, 2019

pomadchin left a comment

pomadchin Nov 22, 2019 •

edited

Loading

echeipesh Nov 22, 2019

pomadchin Nov 22, 2019

pomadchin Nov 22, 2019

pomadchin Nov 22, 2019

echeipesh Nov 22, 2019

pomadchin Nov 22, 2019

echeipesh Nov 22, 2019

pomadchin Nov 22, 2019

echeipesh Nov 22, 2019

pomadchin Nov 22, 2019

echeipesh Nov 22, 2019

pomadchin Nov 22, 2019

Switch to scala 2.12 as build default #3132

Switch to scala 2.12 as build default #3132

Conversation

echeipesh commented Oct 21, 2019 • edited Loading

pomadchin commented Oct 23, 2019 • edited Loading

pomadchin commented Nov 21, 2019

pomadchin left a comment

Choose a reason for hiding this comment

pomadchin Nov 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

echeipesh commented Oct 21, 2019 •

edited

Loading

pomadchin commented Oct 23, 2019 •

edited

Loading

pomadchin Nov 22, 2019 •

edited

Loading