[SPARK-17645][MLLIB][ML][FOLLOW-UP] document minor change #16434
Conversation
|
Test build #70721 has finished for PR 16434 at commit
|
jkbradley
left a comment
jkbradley
left a comment
There was a problem hiding this comment.
Thanks for the PR! Responding to your comment in the other PR, I do think it's worth creating a new test set (or replacing the current one?) which lets us test FDR.
python/pyspark/ml/feature.py
Outdated
| Chi-Squared feature selection, which selects categorical features to use for predicting a | ||
| categorical label. | ||
| Creates a ChiSquared feature selector. |
There was a problem hiding this comment.
This line is now duplicated info, so I'd remove it.
|
Also, could you please change the PR description to be self-contained (rather than just referencing another PR)? The description becomes the commit message. |
|
Test build #70744 has finished for PR 16434 at commit
|
|
Hi @jkbradley , I have updated this PR per your comments. Thanks. |
| chisq.test(x2,y) | ||
| chisq.test(x3,y) | ||
| /* | ||
| * Contingency tables |
There was a problem hiding this comment.
It's kind of nice seeing these tables, but for validation, it's much easier for users to check using R. Could you please include a section for verifying the results with R, like there was before?
|
Thanks @mpjlu ! The changes look good, except that I'd like to have a code snippet for verifying with R. |
|
Test build #70957 has finished for PR 16434 at commit
|
|
Thanks @jkbradley @srowen , I have added a code snippet for verifying with R. |
|
Merged to master |
## What changes were proposed in this pull request? Add FDR test case in ml/feature/ChiSqSelectorSuite. Improve some comments in the code. This is a follow-up pr for apache#15212. ## How was this patch tested? ut Author: Peng, Meng <peng.meng@intel.com> Closes apache#16434 from mpjlu/fdr_fwe_update.
## What changes were proposed in this pull request? Add FDR test case in ml/feature/ChiSqSelectorSuite. Improve some comments in the code. This is a follow-up pr for apache#15212. ## How was this patch tested? ut Author: Peng, Meng <peng.meng@intel.com> Closes apache#16434 from mpjlu/fdr_fwe_update.
4 participants
What changes were proposed in this pull request?
Add FDR test case in ml/feature/ChiSqSelectorSuite.
Improve some comments in the code.
This is a follow-up pr for #15212.
How was this patch tested?
ut