[SPARK-29116][PYTHON][ML] Refactor py classes related to DecisionTree #25929

huaxingao · 2019-09-25T18:50:54Z

What changes were proposed in this pull request?

Move tree related classes to a separate file tree.py
add method predictLeaf in DecisionTreeModel& TreeEnsembleModel

Why are the changes needed?

keep parity between scala and python
easy code maintenance

Does this PR introduce any user-facing change?

Yes
add method predictLeaf in DecisionTreeModel& TreeEnsembleModel
add setMinWeightFractionPerNode in DecisionTreeClassifier and DecisionTreeRegressor

How was this patch tested?

add some doc tests

…arams

SparkQA · 2019-09-25T19:15:58Z

Test build #111364 has finished for PR 25929 at commit b1c418a.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class DecisionTreeClassifierParams(DecisionTreeParams, TreeClassifierParams):

SparkQA · 2019-09-26T18:20:20Z

Test build #111440 has finished for PR 25929 at commit b722aae.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class _DecisionTreeClassifierParams(DecisionTreeParams, TreeClassifierParams):
class DecisionTreeClassifier(JavaProbabilisticClassifier, _DecisionTreeClassifierParams,
class _RandomForestClassifierParams(RandomForestParams, TreeClassifierParams):
class RandomForestClassifier(JavaProbabilisticClassifier, _RandomForestClassifierParams,
class _DecisionTreeRegressorParams(DecisionTreeParams, TreeRegressorParams, HasVarianceCol):
class DecisionTreeRegressor(JavaPredictor, _DecisionTreeRegressorParams, JavaMLWritable,
class DecisionTreeRegressionModel(DecisionTreeModel, _DecisionTreeRegressorParams,
class _RandomForestRegressorParams(RandomForestParams, TreeRegressorParams):
class RandomForestRegressor(JavaPredictor, _RandomForestRegressorParams, JavaMLWritable,
class RandomForestRegressionModel(TreeEnsembleModel, _RandomForestRegressorParams,

SparkQA · 2019-10-03T22:27:33Z

Test build #111756 has finished for PR 25929 at commit 61f318b.

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

SparkQA · 2019-10-03T22:52:23Z

Test build #111759 has finished for PR 25929 at commit 7034a5e.

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

zhengruifeng · 2019-10-08T02:05:21Z

retest this please

SparkQA · 2019-10-08T02:31:50Z

Test build #111863 has finished for PR 25929 at commit 7034a5e.

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

zhengruifeng · 2019-10-08T02:56:10Z

python/pyspark/ml/regression.py

@@ -1252,9 +999,29 @@ def featureImportances(self):
        return self._call_java("featureImportances")


+class GBTRegressorParams(GBTParams, TreeRegressorParams):


What about rename it _GBT...? like above _RandomForestClassifierParams

zhengruifeng · 2019-10-08T02:56:46Z

python/pyspark/ml/tree.py

+        return self._call_java("toString")
+
+
+class DecisionTreeParams(HasCheckpointInterval, HasSeed, HasWeightCol):


zhengruifeng · 2019-10-08T02:57:19Z

python/pyspark/ml/tree.py

+
+
+@inherit_doc
+class DecisionTreeModel(JavaPredictionModel):


zhengruifeng · 2019-10-08T02:57:32Z

python/pyspark/ml/tree.py

+
+
+@inherit_doc
+class TreeEnsembleModel(JavaPredictionModel):


zhengruifeng · 2019-10-08T02:57:58Z

python/pyspark/ml/tree.py

+        return self._call_java("toString")
+
+
+class TreeEnsembleParams(DecisionTreeParams):


zhengruifeng · 2019-10-08T02:58:09Z

python/pyspark/ml/tree.py

+        return self.getOrDefault(self.featureSubsetStrategy)
+
+
+class RandomForestParams(TreeEnsembleParams):


zhengruifeng · 2019-10-08T02:58:21Z

python/pyspark/ml/tree.py

+        return self.getOrDefault(self.impurity)
+
+
+class TreeClassifierParams(object):


zhengruifeng · 2019-10-08T02:58:30Z

python/pyspark/ml/tree.py

+        return self.getOrDefault(self.impurity)
+
+
+class TreeRegressorParams(HasVarianceImpurity):


huaxingao · 2019-10-09T04:58:51Z

@zhengruifeng
Thanks for your comments.
I didn't add _single_leading_underscore for classes that are used for other packages.
I am a little fuzzy about this _single_leading_underscore usage:
In https://pep8.org/#descriptive-naming-styles, it has
_single_leading_underscore: weak “internal use” indicator. E.g. from M import * does not import objects whose name starts with an underscore.
It makes me feel that the class with _single_leading_underscore is for internal use only. It is not intended to be used in other packages. However, if I explicitly import the _single_leading_underscore class, it works OK.
For example, If I do
from pyspark.ml.tree import *, the _single_leading_underscore class is not imported.
If I do
from pyspark.ml.tree import _DecisionTreeModel, _DecisionTreeParams, these classes are imported OK.

zhengruifeng · 2019-10-09T05:25:22Z

@huaxingao Yes, I can reproduce your case.
The 'private' classes can only be imported explicitly. I guess that is way it is a weak “internal use” indicator.
I think we can add _single_leading_underscore according to the scala side.

huaxingao · 2019-10-09T05:36:52Z

OK. I will add _single_leading_underscore to the classes you mentioned in the comments. Thanks!

zhengruifeng · 2019-10-11T03:54:27Z

retest this please

zhengruifeng · 2019-10-12T02:57:48Z

retest this please

SparkQA · 2019-10-12T03:19:06Z

Test build #111947 has finished for PR 25929 at commit 302e98e.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class _DecisionTreeClassifierParams(_DecisionTreeParams, _TreeClassifierParams):
class DecisionTreeClassificationModel(_DecisionTreeModel, JavaProbabilisticClassificationModel,
class _RandomForestClassifierParams(_RandomForestParams, _TreeClassifierParams):
class RandomForestClassificationModel(_TreeEnsembleModel, JavaProbabilisticClassificationModel,
class GBTClassifierParams(_GBTParams, _HasVarianceImpurity):
class GBTClassificationModel(_TreeEnsembleModel, JavaProbabilisticClassificationModel,
class _DecisionTreeRegressorParams(_DecisionTreeParams, _TreeRegressorParams, HasVarianceCol):
class DecisionTreeRegressionModel(_DecisionTreeModel, _DecisionTreeRegressorParams,
class _RandomForestRegressorParams(_RandomForestParams, _TreeRegressorParams):
class RandomForestRegressionModel(_TreeEnsembleModel, _RandomForestRegressorParams,
class _GBTRegressorParams(_GBTParams, _TreeRegressorParams):
class GBTRegressor(JavaPredictor, _GBTRegressorParams, JavaMLWritable, JavaMLReadable):
class GBTRegressionModel(_TreeEnsembleModel, _GBTRegressorParams, JavaMLWritable, JavaMLReadable):
class _DecisionTreeModel(JavaPredictionModel):
class _DecisionTreeParams(HasCheckpointInterval, HasSeed, HasWeightCol):
class _TreeEnsembleModel(JavaPredictionModel):
class _TreeEnsembleParams(_DecisionTreeParams):
class _RandomForestParams(_TreeEnsembleParams):
class _GBTParams(_TreeEnsembleParams, HasMaxIter, HasStepSize, HasValidationIndicatorCol):
class _HasVarianceImpurity(Params):
class _TreeClassifierParams(object):
class _TreeRegressorParams(_HasVarianceImpurity):

zhengruifeng · 2019-10-12T14:15:30Z

Merged to master, thanks @huaxingao

huaxingao · 2019-10-12T17:20:15Z

Thanks a lot for your help! @zhengruifeng

huaxingao added 2 commits September 25, 2019 11:25

[SPARK-29116][PYTHON][ML] Refactor py classes related to DecisionTree

4cc6ffb

remove JavaProbabilisticClassifierParams from DecisionTreeClassifierP…

b1c418a

…arams

dongjoon-hyun added ML PYSPARK labels Sep 25, 2019

add _ infront of xxxParams

b722aae

huaxingao added 2 commits October 3, 2019 15:03

change :py:attr: to :py:attr:class for classes

61f318b

add one more line before .. versionadded

7034a5e

zhengruifeng reviewed Oct 8, 2019

View reviewed changes

address comments

302e98e

zhengruifeng approved these changes Oct 12, 2019

View reviewed changes

zhengruifeng closed this in 8136295 Oct 12, 2019

zero323 mentioned this pull request Oct 23, 2019

Sync with changes merged after 6378d4bc06cd1bb1a209bd5fb63d10ef52d75eb4 zero323/pyspark-stubs#230

Closed

47 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-29116][PYTHON][ML] Refactor py classes related to DecisionTree #25929

[SPARK-29116][PYTHON][ML] Refactor py classes related to DecisionTree #25929

huaxingao commented Sep 25, 2019

SparkQA commented Sep 25, 2019

SparkQA commented Sep 26, 2019

SparkQA commented Oct 3, 2019

SparkQA commented Oct 3, 2019

zhengruifeng commented Oct 8, 2019

SparkQA commented Oct 8, 2019

zhengruifeng Oct 8, 2019

zhengruifeng Oct 8, 2019

zhengruifeng Oct 8, 2019

zhengruifeng Oct 8, 2019

zhengruifeng Oct 8, 2019

zhengruifeng Oct 8, 2019

zhengruifeng Oct 8, 2019

zhengruifeng Oct 8, 2019

huaxingao commented Oct 9, 2019

zhengruifeng commented Oct 9, 2019

huaxingao commented Oct 9, 2019

zhengruifeng commented Oct 11, 2019

zhengruifeng commented Oct 12, 2019

SparkQA commented Oct 12, 2019

zhengruifeng commented Oct 12, 2019

huaxingao commented Oct 12, 2019

		@@ -1252,9 +999,29 @@ def featureImportances(self):
		return self._call_java("featureImportances")


		class GBTRegressorParams(GBTParams, TreeRegressorParams):

		return self._call_java("toString")


		class DecisionTreeParams(HasCheckpointInterval, HasSeed, HasWeightCol):

		return self._call_java("toString")


		class TreeEnsembleParams(DecisionTreeParams):

		return self.getOrDefault(self.featureSubsetStrategy)


		class RandomForestParams(TreeEnsembleParams):

		return self.getOrDefault(self.impurity)


		class TreeClassifierParams(object):

		return self.getOrDefault(self.impurity)


		class TreeRegressorParams(HasVarianceImpurity):

[SPARK-29116][PYTHON][ML] Refactor py classes related to DecisionTree #25929

[SPARK-29116][PYTHON][ML] Refactor py classes related to DecisionTree #25929

Conversation

huaxingao commented Sep 25, 2019

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

SparkQA commented Sep 25, 2019

SparkQA commented Sep 26, 2019

SparkQA commented Oct 3, 2019

SparkQA commented Oct 3, 2019

zhengruifeng commented Oct 8, 2019

SparkQA commented Oct 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huaxingao commented Oct 9, 2019

zhengruifeng commented Oct 9, 2019

huaxingao commented Oct 9, 2019

zhengruifeng commented Oct 11, 2019

zhengruifeng commented Oct 12, 2019

SparkQA commented Oct 12, 2019

zhengruifeng commented Oct 12, 2019

huaxingao commented Oct 12, 2019