Skip to content

[SPARK-16863][ML] ProbabilisticClassifier.fit check threshoulds' length #14470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

zhengruifeng
Copy link
Contributor

What changes were proposed in this pull request?

Add threshoulds' length checking for Classifiers which extends ProbabilisticClassifier

How was this patch tested?

unit tests and manual tests

@SparkQA
Copy link

SparkQA commented Aug 3, 2016

Test build #63146 has finished for PR 14470 at commit 2c6929d.

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

@@ -84,6 +84,13 @@ class DecisionTreeClassifier @Since("1.4.0") (
val categoricalFeatures: Map[Int, Int] =
MetadataUtils.getCategoricalFeatures(dataset.schema($(featuresCol)))
val numClasses: Int = getNumClasses(dataset)

if (isDefined(thresholds)) {
require($(thresholds).length == numClasses, this.getClass.getSimpleName +
Copy link
Member

Choose a reason for hiding this comment

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

Nit, but why not use string interpolation for the class name bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because ProbabilisticClassificationModel.transform first check like this, so I just follow this style.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, OK. I suppose I'm indifferent then.

@zhengruifeng zhengruifeng force-pushed the classifier_check_setThreshoulds_length branch from 2c6929d to df5af72 Compare August 4, 2016 05:52
@SparkQA
Copy link

SparkQA commented Aug 4, 2016

Test build #63213 has finished for PR 14470 at commit df5af72.

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

asfgit pushed a commit that referenced this pull request Aug 4, 2016
## What changes were proposed in this pull request?

Add threshoulds' length checking for Classifiers which extends ProbabilisticClassifier

## How was this patch tested?

unit tests and manual tests

Author: Zheng RuiFeng <ruifengz@foxmail.com>

Closes #14470 from zhengruifeng/classifier_check_setThreshoulds_length.

(cherry picked from commit 0e2e5d7)
Signed-off-by: Sean Owen <sowen@cloudera.com>
@srowen
Copy link
Member

srowen commented Aug 4, 2016

Merged to master/2.0

@asfgit asfgit closed this in 0e2e5d7 Aug 4, 2016
@zhengruifeng zhengruifeng deleted the classifier_check_setThreshoulds_length branch August 5, 2016 01:24
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.

3 participants