Skip to content

Conversation

@mgaido91
Copy link
Contributor

What changes were proposed in this pull request?

The PR removes the deprecated method computeCost of KMeans.

How was this patch tested?

NA

@SparkQA
Copy link

SparkQA commented Oct 29, 2018

Test build #98201 has finished for PR 22875 at commit 9ee93b1.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@mgaido91
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Oct 29, 2018

Test build #98210 has finished for PR 22875 at commit 9ee93b1.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 29, 2018

Test build #98212 has finished for PR 22875 at commit 62f8606.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@mgaido91
Copy link
Contributor Author

mgaido91 commented Nov 5, 2018

cc @holdenk @srowen

@SparkQA
Copy link

SparkQA commented Nov 10, 2018

Test build #98680 has finished for PR 22875 at commit c2fdddc.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Nov 15, 2018

Test build #98874 has finished for PR 22875 at commit 6e2d501.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@mgaido91
Copy link
Contributor Author

any commetn on this @srowen @holdenk ?

@SparkQA
Copy link

SparkQA commented Nov 20, 2018

Test build #99048 has finished for PR 22875 at commit f09e985.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@mgaido91
Copy link
Contributor Author

retest this please

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Just grepping for computeCost -- do any examples need updating? I see some instances but they may be a different computeCost.

But what about BisectingKMeans too?

@mgaido91
Copy link
Contributor Author

Yes, the problem is that there is also the computeCost of BisectingKMeans. I proposed to deprecate it in 2.4 and remove in 3.0, but I didn't manage to have it done for 2.4 (please refer to the discussions on #22764 and #22756 for the details). So computeCost of BisectingKMeans cannot be removed in 3.0 (unfortunately). The examples for KMeans have already been updated using ClusteringEvaluator in #19676.

@srowen
Copy link
Member

srowen commented Nov 20, 2018

How about BisecingKMeansModel? That's what I was looking at. That computeCost is also deprecated.

@mgaido91
Copy link
Contributor Author

Yes, it was deprecated in #22756 and it is deprecated since 3.0, so we cannot remove it...

@srowen
Copy link
Member

srowen commented Nov 20, 2018

Ah OK I wasn't reading that carefully.

@SparkQA
Copy link

SparkQA commented Nov 20, 2018

Test build #99057 has finished for PR 22875 at commit f09e985.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Apologies for the merge conflict now, but looks OK otherwise.

@mgaido91
Copy link
Contributor Author

thanks for your review @srowen

@SparkQA
Copy link

SparkQA commented Nov 21, 2018

Test build #99109 has finished for PR 22875 at commit 2b18e55.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@mgaido91
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Nov 21, 2018

Test build #99123 has finished for PR 22875 at commit 2b18e55.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@mgaido91
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Nov 21, 2018

Test build #99133 has finished for PR 22875 at commit 2b18e55.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

OK by me; apologies for another rebase though. We're merging lots of changes that need MiMa exclusions and it keeps conflicting.

@mgaido91
Copy link
Contributor Author

sure, thanks @srowen , no need to apologize at all, thanks for your help reviewing this.

@SparkQA
Copy link

SparkQA commented Nov 22, 2018

Test build #99174 has finished for PR 22875 at commit 692ff63.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • class RuleSummary(
  • class QueryPlanningTracker
  • class QueryExecution(

@srowen
Copy link
Member

srowen commented Nov 22, 2018

Merged to master

@asfgit asfgit closed this in dd8c179 Nov 22, 2018
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
## What changes were proposed in this pull request?

The PR removes the deprecated method `computeCost` of `KMeans`.

## How was this patch tested?

NA

Closes apache#22875 from mgaido91/SPARK-25867.

Authored-by: Marco Gaido <marcogaido91@gmail.com>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants