Skip to content

[SPARK-19899][ML] Replace featuresCol with itemsCol in ml.fpm.FPGrowth - #17321

Closed
zero323 wants to merge 1 commit into
apache:masterfrom
zero323:SPARK-19899
Closed

[SPARK-19899][ML] Replace featuresCol with itemsCol in ml.fpm.FPGrowth#17321
zero323 wants to merge 1 commit into
apache:masterfrom
zero323:SPARK-19899

Conversation

@zero323

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

Replaces featuresColParam with itemsCol. See SPARK-19899.

How was this patch tested?

Manual tests. Existing unit tests.

@SparkQA

Copy link
Copy Markdown

Test build #74684 has finished for PR 17321 at commit 77bf12d.

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

* Default: "items"
* @group param
*/
@Since("2.2.0")

@MLnickMLnickMar 17, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We normally don't annotate in traits since later things may be overridden by concrete implementers. Although I do see a few places where it is done...

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

FPGrowthParams has been annotated before so I decided to make it consistent. I can remove annotation here, but then we should probably follow through and remove the rest.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's OK to have it here. Only FPGrowth and FPGrowthModel will ever inherit from FPGrowthParams.

@jkbradley

Copy link
Copy Markdown
Member

LGTM
Thanks for the PR!
Merging with master

@zero323

Copy link
Copy Markdown
MemberAuthor

Thanks @jkbradley

@zero323
zero323 deleted the SPARK-19899 branch April 6, 2017 10:56
Sign up for freeto 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.

4 participants

@zero323@SparkQA@jkbradley@MLnick