Skip to content

Conversation

@jasoncl
Copy link
Contributor

@jasoncl jasoncl commented Apr 15, 2016

What changes were proposed in this pull request?

Added windowSize getter/setter to ML/MLlib

How was this patch tested?

Added test cases in tests.py under both ML and MLlib

"the minimum number of times a token must appear to be included in the " +
"word2vec model's vocabulary", typeConverter=TypeConverters.toInt)
windowSize = Param(Params._dummy(), "windowSize",
"the window size (context words from [-window, window])",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should mention the default value.

@holdenk
Copy link
Contributor

holdenk commented Apr 18, 2016

Thanks for taking the initiative to do this. A few minor comments from the first pass through, but in the meantime maybe one the admins (possibly @jkbradley) could either say ok to jenkins to test or add to the whitelist?

@MLnick
Copy link
Contributor

MLnick commented Apr 18, 2016

ok to test


def test_word2vec_param(self):
model = Word2Vec() \
.setWindowSize(6)
Copy link
Contributor

Choose a reason for hiding this comment

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

minor, but this can be one line

@MLnick
Copy link
Contributor

MLnick commented Apr 18, 2016

One minor comment. Pending that, the tests and @holdenk's comments, LGTM.

@holdenk
Copy link
Contributor

holdenk commented Apr 18, 2016

LGTM pending tests :)

@SparkQA
Copy link

SparkQA commented Apr 18, 2016

Test build #56080 has finished for PR 12428 at commit 5cdcf22.

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

@jkbradley
Copy link
Member

LGTM
Thanks for the PR & others for reviewing!
Merging with master

@asfgit asfgit closed this in 3d66a2c Apr 18, 2016
lw-lin pushed a commit to lw-lin/spark that referenced this pull request Apr 20, 2016
…ze method

## What changes were proposed in this pull request?
Added windowSize getter/setter to ML/MLlib

## How was this patch tested?
Added test cases in tests.py under both ML and MLlib

Author: Jason Lee <[email protected]>

Closes apache#12428 from jasoncl/SPARK-14564.
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