Skip to content
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

[SPARK-29381][FOLLOWUP][PYTHON][ML] Add 'private' _XXXParams classes for classification & regression #26142

Closed
wants to merge 1 commit into from

Conversation

huaxingao
Copy link
Contributor

What changes were proposed in this pull request?

Add private _XXXParams classes for classification & regression

Why are the changes needed?

To keep parity between scala and python

Does this PR introduce any user-facing change?

Yes. Add gettters/setters for the following Model classes

LinearSVCModel:
get/setRegParam
get/setMaxIte
get/setFitIntercept
get/setTol
get/setStandardization
get/setWeightCol
get/setAggregationDepth
get/setThreshold

LogisticRegressionModel:
get/setRegParam
get/setElasticNetParam
get/setMaxIter
get/setFitIntercept
get/setTol
get/setStandardization
get/setWeightCol
get/setAggregationDepth
get/setThreshold

NaiveBayesModel:
get/setWeightCol

LinearRegressionModel:
get/setRegParam
get/setElasticNetParam
get/setMaxIter
get/setTol
get/setFitIntercept
get/setStandardization
get/setWeight
get/setSolver
get/setAggregationDepth
get/setLoss

GeneralizedLinearRegressionModel:
get/setFitIntercept
get/setMaxIter
get/setTol
get/setRegParam
get/setWeightCol
get/setSolver

How was this patch tested?

Add a few doctest

"""
self._set(thresholds=value)
self._clear(self.threshold)
return self
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a little strange to have setThreshold/Thresholds in the XXXParams class, but scala LogisticRegressionParams does this way, so I just do the same to be consistent with scala side.

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Are there any actual changes to the API or it's just moving definitions around?

@SparkQA
Copy link

SparkQA commented Oct 16, 2019

Test build #112181 has finished for PR 26142 at commit 30c4b24.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • class _LinearSVCParams(_JavaClassifierParams, HasRegParam, HasMaxIter, HasFitIntercept, HasTol,
  • class LinearSVC(JavaClassifier, _LinearSVCParams, JavaMLWritable, JavaMLReadable):
  • class LinearSVCModel(JavaClassificationModel, _LinearSVCParams, JavaMLWritable, JavaMLReadable):
  • class _LogisticRegressionParams(_JavaProbabilisticClassifierParams, HasRegParam,
  • class LogisticRegression(JavaProbabilisticClassifier, _LogisticRegressionParams, JavaMLWritable,
  • class LogisticRegressionModel(JavaProbabilisticClassificationModel, _LogisticRegressionParams,
  • class _NaiveBayesParams(_JavaPredictorParams, HasWeightCol):
  • class NaiveBayes(JavaProbabilisticClassifier, _NaiveBayesParams, HasThresholds, HasWeightCol,
  • class NaiveBayesModel(JavaProbabilisticClassificationModel, _NaiveBayesParams, JavaMLWritable,
  • class _MultilayerPerceptronParams(_JavaProbabilisticClassifierParams, HasSeed, HasMaxIter,
  • class MultilayerPerceptronClassifier(JavaProbabilisticClassifier, _MultilayerPerceptronParams,
  • class _LinearRegressionParams(_JavaPredictorParams, HasRegParam, HasElasticNetParam, HasMaxIter,
  • class LinearRegression(JavaPredictor, _LinearRegressionParams, JavaMLWritable, JavaMLReadable):
  • class LinearRegressionModel(JavaPredictionModel, _LinearRegressionParams, GeneralJavaMLWritable,
  • class _IsotonicRegressionParams(HasFeaturesCol, HasLabelCol, HasPredictionCol, HasWeightCol):
  • class IsotonicRegression(JavaEstimator, _IsotonicRegressionParams, HasWeightCol,
  • class IsotonicRegressionModel(JavaModel, _IsotonicRegressionParams, JavaMLWritable,
  • class _GeneralizedLinearRegressionParams(_JavaPredictorParams, HasFitIntercept, HasMaxIter,
  • class GeneralizedLinearRegression(JavaPredictor, _GeneralizedLinearRegressionParams,
  • class GeneralizedLinearRegressionModel(JavaPredictionModel, _GeneralizedLinearRegressionParams,

@zhengruifeng
Copy link
Contributor

@srowen This PR should add above-mentioned methods. (Some of them are redundant setters, we will remove in following PR)

@huaxingao some conflicts happened

@huaxingao
Copy link
Contributor Author

@srowen sorry, I somehow didn't see your comment. As @zhengruifeng mentioned, this PR adds setters/getters in LinearSVCModel, LogisticRegressionModel, NaiveBayesModel,
LinearRegressionModel and GeneralizedLinearRegressionModel. The redundant setters will be remove later in the last parity PR.

@zhengruifeng I will rebase and resolve the conflict.

@SparkQA
Copy link

SparkQA commented Oct 18, 2019

Test build #112253 has finished for PR 26142 at commit cdf87c7.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • class _LinearSVCParams(_JavaClassifierParams, HasRegParam, HasMaxIter, HasFitIntercept, HasTol,
  • class LinearSVC(JavaClassifier, _LinearSVCParams, JavaMLWritable, JavaMLReadable):
  • class LinearSVCModel(JavaClassificationModel, _LinearSVCParams, JavaMLWritable, JavaMLReadable):
  • class _LogisticRegressionParams(_JavaProbabilisticClassifierParams, HasRegParam,
  • class LogisticRegression(JavaProbabilisticClassifier, _LogisticRegressionParams, JavaMLWritable,
  • class LogisticRegressionModel(JavaProbabilisticClassificationModel, _LogisticRegressionParams,
  • class _NaiveBayesParams(_JavaPredictorParams, HasWeightCol):
  • class NaiveBayes(JavaProbabilisticClassifier, _NaiveBayesParams, HasThresholds, HasWeightCol,
  • class NaiveBayesModel(JavaProbabilisticClassificationModel, _NaiveBayesParams, JavaMLWritable,
  • class _MultilayerPerceptronParams(_JavaProbabilisticClassifierParams, HasSeed, HasMaxIter,
  • class MultilayerPerceptronClassifier(JavaProbabilisticClassifier, _MultilayerPerceptronParams,
  • class _LinearRegressionParams(_JavaPredictorParams, HasRegParam, HasElasticNetParam, HasMaxIter,
  • class LinearRegression(JavaPredictor, _LinearRegressionParams, JavaMLWritable, JavaMLReadable):
  • class LinearRegressionModel(JavaPredictionModel, _LinearRegressionParams, GeneralJavaMLWritable,
  • class _IsotonicRegressionParams(HasFeaturesCol, HasLabelCol, HasPredictionCol, HasWeightCol):
  • class IsotonicRegression(JavaEstimator, _IsotonicRegressionParams, HasWeightCol,
  • class IsotonicRegressionModel(JavaModel, _IsotonicRegressionParams, JavaMLWritable,
  • class _GeneralizedLinearRegressionParams(_JavaPredictorParams, HasFitIntercept, HasMaxIter,
  • class GeneralizedLinearRegression(JavaPredictor, _GeneralizedLinearRegressionParams,
  • class GeneralizedLinearRegressionModel(JavaPredictionModel, _GeneralizedLinearRegressionParams,

@zhengruifeng
Copy link
Contributor

merged to master, thanks all

@huaxingao
Copy link
Contributor Author

Thanks! @zhengruifeng @srowen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants