Skip to content

Conversation

@mengxr
Copy link
Contributor

@mengxr mengxr commented Mar 22, 2016

What changes were proposed in this pull request?

This PR continues the work in #11486 from @yinxusen with some code refactoring. In R package e1071, naiveBayes supports both categorical (Bernoulli) and continuous features (Gaussian), while in MLlib we support Bernoulli and multinomial. This PR implements the common subset: Bernoulli.

I moved the implementation out from SparkRWrappers to NaiveBayesWrapper to make it easier to read. Argument names, default values, and summary now match e1071's naiveBayes.

I removed the preprocess part that omit NA values because we don't know which columns to process.

How was this patch tested?

Test against output from R package e1071's naiveBayes.

cc: @yanboliang @yinxusen

Closes #11486

@SparkQA
Copy link

SparkQA commented Mar 22, 2016

Test build #53781 has finished for PR 11890 at commit 43e6fa5.

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

@SparkQA
Copy link

SparkQA commented Mar 22, 2016

Test build #53782 has finished for PR 11890 at commit b3312c7.

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

@SparkQA
Copy link

SparkQA commented Mar 22, 2016

Test build #53783 has finished for PR 11890 at commit 12a41bb.

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

@mengxr mengxr changed the title [SPARK-13449][MLLIB][R] Naive Bayes wrapper in SparkR [SPARK-13449] Naive Bayes wrapper in SparkR Mar 22, 2016
@SparkQA
Copy link

SparkQA commented Mar 22, 2016

Test build #53787 has finished for PR 11890 at commit 0ac224e.

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

@thunterdb
Copy link
Contributor

@mengxr it looks great, I have no comment

@mengxr
Copy link
Contributor Author

mengxr commented Mar 22, 2016

Thanks! Merged into master.

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.

4 participants