You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by yanboliang <gi...@git.apache.org> on 2017/05/05 04:14:22 UTC
[GitHub] spark pull request #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated me...
GitHub user yanboliang opened a pull request:
https://github.com/apache/spark/pull/17867
[SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods for ML
## What changes were proposed in this pull request?
Remove ML methods we deprecated in 2.1.
## How was this patch tested?
Existing tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yanboliang/spark spark-20606
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/17867.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 #17867
----
commit e5b1337f3a09995568b69dde83dba90f9c01fcfe
Author: Yanbo Liang <yb...@gmail.com>
Date: 2017-05-05T03:18:53Z
Remove deprecated methods for ML.
commit 4922c03a0ba0ed7386198b3e9e068352cc4378f5
Author: Yanbo Liang <yb...@gmail.com>
Date: 2017-05-05T04:12:02Z
Add stuff to MimaExcludes.
----
---
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/17867
Hm, isn't it sudden to remove deprecated methods after 1 minor release?
Also why does this not remove the methods from subclasses? if they're still needed by implementations, then does it make sense to deprecate/remove them?
---
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17867
**[Test build #76479 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/76479/testReport)** for PR 17867 at commit [`4922c03`](https://github.com/apache/spark/commit/4922c03a0ba0ed7386198b3e9e068352cc4378f5).
* 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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17867
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on the issue:
https://github.com/apache/spark/pull/17867
I'll merge this to catch 2.2, and update user guide to reflect this change as usual, since this is one of the ML QA tasks which blocks the 2.2 release. If there are more comments, I can address them in follow-up work. Thanks.
---
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on the issue:
https://github.com/apache/spark/pull/17867
I'm OK to revert this change and change the deprecation comments to say the items will be removed in ```3.0.0``` instead of ```2.x```. Will do them in two follow-up PRs. Thanks.
---
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on the issue:
https://github.com/apache/spark/pull/17867
@srowen These methods were deprecated when 2.1 releasing, and we usually remove methods deprecated in last feature/minor release (when 2.1 releasing, we remove methods deprecated in 2.0, see #15913). We only remove the setter methods from the trait, which cause them be remove from the model classes who extend them. Since it does not make sense to have it in the model classes, and actually we never use them and mark them deprecated for one minor release cycle. You can refer the discussion [here](https://github.com/apache/spark/pull/15913#discussion_r89662469) . Thanks.
---
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on the issue:
https://github.com/apache/spark/pull/17867
Yeah, that's because we didn't always do that in time, especially for spark.mllib packages which has been in maintenance mode. But for spark.ml package, I think we always keep to remove methods deprecated in last feature/minor release. And we have already warned ```This method is deprecated and will be removed in 2.2.0``` in API documents, it may confuse users if we don't take action follow the API annotation. I'm prefer to merge this as the old convention, and start the new convention in the following release which we should add annotation like ```deprecated and will be removed in 3.0.0```. What do you think about it? Thanks.
---
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on the issue:
https://github.com/apache/spark/pull/17867
I sent #17946 to change the deprecation comments to say the items will be removed in 3.0.0 instead of 2.x, please feel free to review and comments. @srowen @jkbradley
---
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:
https://github.com/apache/spark/pull/17867
I actually think @srowen has a good point that we should maintain more stable minor releases. I'd be in support of reverting this patch and changing the deprecation comments to say the items will be removed in "3.0.0" instead of 2.x minor versions. Is that OK with you @yanboliang ?
Note: I also noticed that MLWriter.context does *not* appear as deprecated in the API docs because the deprecated implementation is overridden with a non-deprecated method. (That was my bad for committing that PR.)
---
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/17867
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/76479/
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by yanboliang <gi...@git.apache.org>.
Github user yanboliang commented on the issue:
https://github.com/apache/spark/pull/17867
@srowen Any more comments?
---
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/17867
I do see methods in the code that are deprecated yet retained for a few minor releases -- generally I'd expect we only actually remove them in a major release because it is a breaking change for any caller. I guess that's my primary question: is this binary/source compatible?
---
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated methods f...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/17867
**[Test build #76479 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/76479/testReport)** for PR 17867 at commit [`4922c03`](https://github.com/apache/spark/commit/4922c03a0ba0ed7386198b3e9e068352cc4378f5).
---
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 #17867: [SPARK-20606][ML] ML 2.2 QA: Remove deprecated me...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/17867
---
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