You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by VinceShieh <gi...@git.apache.org> on 2016/09/12 06:55:16 UTC
[GitHub] spark pull request #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spa...
GitHub user VinceShieh opened a pull request:
https://github.com/apache/spark/pull/15055
[SPARK-17462][MLLIB]use VersionUtils to parse Spark version strings
## What changes were proposed in this pull request?
Several places in MLlib use custom regexes or other approaches to parse Spark versions.
Those should be fixed to use the VersionUtils. This PR replaces custom regexes with
VersionUtils to get Spark version numbers.
## How was this patch tested?
Existing tests.
Signed-off-by: VinceShieh <vi...@intel.com>
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/VinceShieh/spark SPARK-17462
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/15055.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #15055
----
commit 72a87b0f8a386ff53903620c3cddee6744c73a96
Author: VinceShieh <vi...@intel.com>
Date: 2016-09-12T06:37:42Z
[SPARK-17462][MLLIB]use VersionUtils to parse Spark version strings
Several places in MLlib use custom regexes or other approaches. Those should be
fixed to use the VersionUtils
Signed-off-by: VinceShieh <vi...@intel.com>
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15055
**[Test build #3430 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3430/consoleFull)** for PR 15055 at commit [`72a87b0`](https://github.com/apache/spark/commit/72a87b0f8a386ff53903620c3cddee6744c73a96).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spa...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/15055
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15055
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/68768/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15055
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65241/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/15055
Merged to master/2.1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15055
**[Test build #68768 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68768/consoleFull)** for PR 15055 at commit [`b29b9be`](https://github.com/apache/spark/commit/b29b9be7aa20761a478551a4d812b748287560aa).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15055
**[Test build #3430 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3430/consoleFull)** for PR 15055 at commit [`72a87b0`](https://github.com/apache/spark/commit/72a87b0f8a386ff53903620c3cddee6744c73a96).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spa...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/15055#discussion_r88428343
--- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/PCA.scala ---
@@ -34,6 +34,7 @@ import org.apache.spark.rdd.RDD
import org.apache.spark.sql._
import org.apache.spark.sql.functions._
import org.apache.spark.sql.types.{StructField, StructType}
+import org.apache.spark.util.VersionUtils._
--- End diff --
You can leave it. In general I think that when you find `foo()` in code one looks for `foo` in the same compilation unit, and it's a little distracting to find it was actually imported by itself. IDEs figure it out, sure, but this mechanism is really for things like importing a method used a lot or something. Anyway, we don't have a consistent approach in the code, so OK.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15055
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by VinceShieh <gi...@git.apache.org>.
Github user VinceShieh commented on the issue:
https://github.com/apache/spark/pull/15055
@srowen @jkbradley do you have time to take a look at this one?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spa...
Posted by VinceShieh <gi...@git.apache.org>.
Github user VinceShieh commented on a diff in the pull request:
https://github.com/apache/spark/pull/15055#discussion_r88376643
--- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/PCA.scala ---
@@ -34,6 +34,7 @@ import org.apache.spark.rdd.RDD
import org.apache.spark.sql._
import org.apache.spark.sql.functions._
import org.apache.spark.sql.types.{StructField, StructType}
+import org.apache.spark.util.VersionUtils._
--- End diff --
@srowen Sure, these are what I can find in ml/mllib.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15055
**[Test build #65241 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65241/consoleFull)** for PR 15055 at commit [`72a87b0`](https://github.com/apache/spark/commit/72a87b0f8a386ff53903620c3cddee6744c73a96).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15055
**[Test build #68768 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/68768/consoleFull)** for PR 15055 at commit [`b29b9be`](https://github.com/apache/spark/commit/b29b9be7aa20761a478551a4d812b748287560aa).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spa...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/15055#discussion_r88207895
--- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/PCA.scala ---
@@ -34,6 +34,7 @@ import org.apache.spark.rdd.RDD
import org.apache.spark.sql._
import org.apache.spark.sql.functions._
import org.apache.spark.sql.types.{StructField, StructType}
+import org.apache.spark.util.VersionUtils._
--- End diff --
In general I think we prefer importing classes rather than importing members, except where the latter is clearly clearer. I'm not sure it is here. It's minor either way.
Are there any other occurrences across the code? i want to try to solve these issues in one go.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spa...
Posted by VinceShieh <gi...@git.apache.org>.
Github user VinceShieh commented on a diff in the pull request:
https://github.com/apache/spark/pull/15055#discussion_r88427044
--- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/PCA.scala ---
@@ -34,6 +34,7 @@ import org.apache.spark.rdd.RDD
import org.apache.spark.sql._
import org.apache.spark.sql.functions._
import org.apache.spark.sql.types.{StructField, StructType}
+import org.apache.spark.util.VersionUtils._
--- End diff --
hmm, personally I think it's fine to keep it this way, though it's only 'majorVersion' needed here, so we can import org.apache.spark.util.VersionUtils.majorVersion instead. Sounds good to you? @srowen
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15055
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15055: [SPARK-17462][MLLIB]use VersionUtils to parse Spark vers...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15055
**[Test build #65241 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65241/consoleFull)** for PR 15055 at commit [`72a87b0`](https://github.com/apache/spark/commit/72a87b0f8a386ff53903620c3cddee6744c73a96).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org