Skip to content
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

[SPARK-45742][CORE][CONNECT][MLLIB][PYTHON] Introduce an implicit function for Scala Array to wrap into immutable.ArraySeq. #43607

Closed
wants to merge 6 commits into from

Conversation

LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Oct 31, 2023

What changes were proposed in this pull request?

Currently, we need to use immutable.ArraySeq.unsafeWrapArray(array) to wrap an Array into an immutable.ArraySeq, which makes the code look bloated.

So this PR introduces an implicit function toImmutableArraySeq to make it easier for Scala Array to be wrapped into immutable.ArraySeq.

After this pr, we can use the following way to wrap an array into an immutable.ArraySeq:

import org.apache.spark.util.ArrayImplicits._

val dataArray = ...
val immutableArraySeq = dataArray.toImmutableArraySeq

At the same time, this pr replaces the existing use of immutable.ArraySeq.unsafeWrapArray(array) with the new method.

On the other hand, this implicit function will be conducive to the progress of work SPARK-45686 and SPARK-45687.

Why are the changes needed?

Makes the code for wrapping a Scala Array into an immutable.ArraySeq look less bloated.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GitHub Actions

Was this patch authored or co-authored using generative AI tooling?

No

@LuciferYang LuciferYang changed the title [SPARK-45742][CORE][CONNECT][MLLIB] Introduce an implicit function for Scala Array to wrap into immutable.ArraySeq. [SPARK-45742][SQL][CONNECT][MLLIB][PYTHON] Introduce an implicit function for Scala Array to wrap into immutable.ArraySeq. Oct 31, 2023
@LuciferYang LuciferYang changed the title [SPARK-45742][SQL][CONNECT][MLLIB][PYTHON] Introduce an implicit function for Scala Array to wrap into immutable.ArraySeq. [SPARK-45742][CORE][CONNECT][MLLIB][PYTHON] Introduce an implicit function for Scala Array to wrap into immutable.ArraySeq. Oct 31, 2023
* Wraps an Array[T] as an immutable.ArraySeq[T] without copying.
*/
def toImmutableArraySeq: immutable.ArraySeq[T] =
if (xs eq null) null
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Welcome to Scala 2.13.12 (OpenJDK 64-Bit Server VM, Java 17.0.8).
Type in expressions for evaluation. Or try :help.

scala> val a: Array[Int] = null
val a: Array[Int] = null

scala> scala.collection.immutable.ArraySeq.unsafeWrapArray(a)
val res0: scala.collection.immutable.ArraySeq[Int] = null

@github-actions github-actions bot added the CORE label Nov 1, 2023
import org.apache.spark.SparkFunSuite
import org.apache.spark.util.ArrayImplicits._

class ArrayImplicitsSuite extends SparkFunSuite {
Copy link
Contributor Author

@LuciferYang LuciferYang Nov 1, 2023

Choose a reason for hiding this comment

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

Although this pr has modified the utils module, looking at the test parameters -Phadoop-3 repl/test avro/test examples/test, it did not execute the tests of the utils module, so it seems that the test of the utils module has not been actually verified on GA, put ArrayImplicitsSuite in the core module for now.

The issue with the utils module test will be followed up separately.

@LuciferYang
Copy link
Contributor Author

Could you help review this pr if you have time. @dongjoon-hyun Thanks ~

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @LuciferYang .

@dongjoon-hyun
Copy link
Member

Merged to master.

@LuciferYang
Copy link
Contributor Author

Thanks @dongjoon-hyun ~

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

Successfully merging this pull request may close these issues.

2 participants