-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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
Conversation
""" | ||
self._set(thresholds=value) | ||
self._clear(self.threshold) | ||
return self |
There was a problem hiding this comment.
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.
There was a problem hiding this 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?
Test build #112181 has finished for PR 26142 at commit
|
@srowen This PR should add above-mentioned methods. (Some of them are redundant setters, we will remove in following PR) @huaxingao some conflicts happened |
@srowen sorry, I somehow didn't see your comment. As @zhengruifeng mentioned, this PR adds setters/getters in @zhengruifeng I will rebase and resolve the conflict. |
…for classification & regression
30c4b24
to
cdf87c7
Compare
Test build #112253 has finished for PR 26142 at commit
|
merged to master, thanks all |
Thanks! @zhengruifeng @srowen |
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
How was this patch tested?
Add a few doctest