[SPARK-23085][ML] API parity for mllib.linalg.Vectors.sparse #20275
Conversation
|
Test build #86158 has finished for PR 20275 at commit
|
|
as far as I know mllib is not maintained anymore, thus I am not sure this fix is useful |
|
That's true, although, there's no conceptual reason not to support a 0-size vector, and the existing checks even checked for >= 0 in one case. This is arguably just fixing internal consistency. How about a unit test? |
1a3cd3a to
ac067e7
Compare
|
Test build #86225 has finished for PR 20275 at commit
|
|
Unless someone like @jkbradley or @MLnick objects, I think this doesn't hurt and actually makes things a little more consistent. |
| @@ -113,6 +113,13 @@ class VectorsSuite extends SparkFunSuite with Logging { | |||
| assert(vec.toArray === arr) | |||
| } | |||
|
|
|||
| test("zero-length sparse vector") { | |||
There was a problem hiding this comment.
We may as well add the same test to ml.linalg
| @@ -113,6 +113,13 @@ class VectorsSuite extends SparkFunSuite with Logging { | |||
| assert(vec.toArray === arr) | |||
| } | |||
|
|
|||
| test("zero-length sparse vector") { | |||
There was a problem hiding this comment.
While we're doing this we may as well also add a test to intercept the exception for negative size (since the other sparse vector construction tests check for the other constraints but a size test is missing), for both ml and mllib
|
Test build #86373 has finished for PR 20275 at commit
|
|
Merged to master |
What changes were proposed in this pull request?
ML.Vectors#sparse(size: Int, elements: Seq[(Int, Double)])support zero-lengthHow was this patch tested?
existing tests