Skip to content

Conversation

@mgaido91
Copy link
Contributor

What changes were proposed in this pull request?

When we added the distanceMeasure, we didn't update the formatVersion for KMeans. Despite this is not a big issue, as that information is used nowhere, we are returning a wrong information.

How was this patch tested?

NA

@mgaido91
Copy link
Contributor Author

@viirya
Copy link
Member

viirya commented Oct 29, 2018

Are we considering to remove it? If no, as save now calls SaveLoadV2_0, it seems good to update this variable.

@mgaido91
Copy link
Contributor Author

@viirya I think we can remove it as it is used nowhere. The point is: even if we remove it, we will remove it only on 3.0, so for 2.4 we should anyway fix this IMO.

@SparkQA
Copy link

SparkQA commented Oct 29, 2018

Test build #98196 has finished for PR 22873 at commit 874c9c0.

  • 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

any comments on this?

@WeichenXu123
Copy link
Contributor

LGTM. Thanks!

@cloud-fan
Copy link
Contributor

thanks, merging to master/2.4!

asfgit pushed a commit that referenced this pull request Nov 6, 2018
## What changes were proposed in this pull request?

When we added the `distanceMeasure`, we didn't update the `formatVersion` for `KMeans`. Despite this is not a big issue, as that information is used nowhere, we are returning a wrong information.

## How was this patch tested?

NA

Closes #22873 from mgaido91/SPARK-25866.

Authored-by: Marco Gaido <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 6b42587)
Signed-off-by: Wenchen Fan <[email protected]>
@asfgit asfgit closed this in 6b42587 Nov 6, 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?

When we added the `distanceMeasure`, we didn't update the `formatVersion` for `KMeans`. Despite this is not a big issue, as that information is used nowhere, we are returning a wrong information.

## How was this patch tested?

NA

Closes apache#22873 from mgaido91/SPARK-25866.

Authored-by: Marco Gaido <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 23, 2019
## What changes were proposed in this pull request?

When we added the `distanceMeasure`, we didn't update the `formatVersion` for `KMeans`. Despite this is not a big issue, as that information is used nowhere, we are returning a wrong information.

## How was this patch tested?

NA

Closes apache#22873 from mgaido91/SPARK-25866.

Authored-by: Marco Gaido <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 6b42587)
Signed-off-by: Wenchen Fan <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Aug 1, 2019
## What changes were proposed in this pull request?

When we added the `distanceMeasure`, we didn't update the `formatVersion` for `KMeans`. Despite this is not a big issue, as that information is used nowhere, we are returning a wrong information.

## How was this patch tested?

NA

Closes apache#22873 from mgaido91/SPARK-25866.

Authored-by: Marco Gaido <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 6b42587)
Signed-off-by: Wenchen Fan <[email protected]>
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.

5 participants