Add support for scala 2.13#417
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## master #417 +/- ##
=======================================
Coverage 91.19% 91.19%
=======================================
Files 18 18
Lines 829 829
Branches 49 49
=======================================
Hits 756 756
Misses 73 73
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| */ | ||
| def landmarks(value: util.ArrayList[Any]): this.type = { | ||
| landmarks(value.asScala) | ||
| landmarks(value.asScala.toSeq) |
There was a problem hiding this comment.
Looking it a bit closer to why this was needed in probably this change in scala 2.13 (https://github.com/scala/scala/releases/tag/v2.13.0)
* Immutable scala.Seq
* Seq is now an alias for collection.immutable.Seq
* Before, it was an alias for the possibly-mutable collection.Seq.
So when the java.util.ArrayList[Any] is converted we end up with a scala.collection.mutable.Buffer[Any]. In scala 2.12 this is a sub type of Seq[Any] while in scala 2.13 it it not (it only a subtype of mutable.Seq). So my fix of doing toSeq probably makes a extra copy. One other solution might be that one change Seq to collection.Seq in the needed places to keep the 2.12 semantics. With this change
diff --git a/src/main/scala/org/graphframes/lib/ShortestPaths.scala b/src/main/scala/org/graphframes/lib/ShortestPaths.scala
index f110d6f..4641ce7 100644
--- a/src/main/scala/org/graphframes/lib/ShortestPaths.scala
+++ b/src/main/scala/org/graphframes/lib/ShortestPaths.scala
@@ -19,6 +19,7 @@ package org.graphframes.lib
import java.util
+import scala.collection
import scala.collection.JavaConverters._
import org.apache.spark.graphx.{lib => graphxlib}
@@ -39,12 +40,12 @@ import org.graphframes.GraphFrame
* the shortest-path distance to each reachable landmark vertex.
*/
class ShortestPaths private[graphframes] (private val graph: GraphFrame) extends Arguments {
- private var lmarks: Option[Seq[Any]] = None
+ private var lmarks: Option[collection.Seq[Any]] = None
/**
* The list of landmark vertex ids. Shortest paths will be computed to each landmark.
*/
- def landmarks(value: Seq[Any]): this.type = {
+ def landmarks(value: collection.Seq[Any]): this.type = {
// TODO(tjh) do some initial checks here, without running queries.
lmarks = Some(value)
this
@@ -54,7 +55,7 @@ class ShortestPaths private[graphframes] (private val graph: GraphFrame) extends
* The list of landmark vertex ids. Shortest paths will be computed to each landmark.
*/
def landmarks(value: util.ArrayList[Any]): this.type = {
- landmarks(value.asScala.toSeq)
+ landmarks(value.asScala)
}
def run(): DataFrame = {
@@ -64,7 +65,7 @@ class ShortestPaths private[graphframes] (private val graph: GraphFrame) extends
private object ShortestPaths {
- private def run(graph: GraphFrame, landmarks: Seq[Any]): DataFrame = {
+ private def run(graph: GraphFrame, landmarks: collection.Seq[Any]): DataFrame = {
val idType = graph.vertices.schema(GraphFrame.ID).dataType
val longIdToLandmark = landmarks.map(l => GraphXConversions.integralId(graph, l) -> l).toMap
val gx = graphxlib.ShortestPaths.run(
it also compile both in 2.12 and 2.13.
|
Can we merge this? |
|
@eejbyfeldt I would be grateful if you could please merge this as my team and I would like to use it |
|
@RRajdev I am not a committer to this project so I do not have the rights needed to merge the PR. We would need the reviewer @WeichenXu123 or some other committer to merge it. |
Only adds scala CI builds as I don't belive there is any pre-built pyspark release using scala 2.13.
eejbyfeldt
force-pushed
the
scala-2.13
branch
from
7bc49db to
80fb0f5
Compare
|
thanks - i will follow up with them and thanks for doing this!
…---- On Tue, 21 Feb 2023 12:06:11 +0000 Emil Ejbyfeldt ***@***.***> wrote ---
https://github.com/RRajdev I am not a committer to this project so I do not have the rights needed to merge the PR. We would need the reviewer https://github.com/WeichenXu123 or some other committer to merge it.
—
Reply to this email directly, #417 (comment), or https://github.com/notifications/unsubscribe-auth/AIPGISWLVQPF35T2EHTNR33WYSVTHANCNFSM57RYZVUA.
You are receiving this because you were mentioned.
|
|
@WeichenXu123 @codecov-commenter @rjurney can this please be merged in as our team could use this change - tia |
|
Cool! |
5 participants
Only adds scala CI builds as I don't belive there is any pre-built
pyspark release using scala 2.13.