FIX ravel prediction of PLSRegression when fitted on 1d y#26602
FIX ravel prediction of PLSRegression when fitted on 1d y#26602OmarManzoor merged 8 commits intoscikit-learn:mainfrom
PLSRegression when fitted on 1d y#26602Conversation
| Ypred = X @ self.coef_.T | ||
| return Ypred + self.intercept_ | ||
| Ypred = X @ self.coef_.T + self.intercept_ | ||
| return Ypred.ravel() if self._predict_1d else Ypred |
There was a problem hiding this comment.
nit: my personal taste/preference is to split this up into a if self._predict_1d: ... else: ... (so spread over four lines). I find it easier to read/spot that there is a if here compared to using the one liner. But others might have other tastes :-/
There was a problem hiding this comment.
In my opinion I think it is fine both ways.
betatim
left a comment
There was a problem hiding this comment.
I think this is a reasonable and minimal solution to the problem
OmarManzoor
left a comment
There was a problem hiding this comment.
Thanks for the PR @Charlie-XIAO. Just a few comments.
PLSRegression when fitted on 1d yPLSRegression when fitted on 1d y
|
@OmarManzoor Thanks for your review! I've resolved the issues you mentioned. |
|
Sorry for the late reply @OmarManzoor, committed your suggestions now. |
OmarManzoor
left a comment
There was a problem hiding this comment.
LGTM! Thanks @Charlie-XIAO
|
The changelog entry of this PR seems to have accidentally been deleted in the recent restructuring. I think it needs to be added again. |
Reference Issues/PRs
Fixes: #26549.
What does this implement/fix? Explain your changes.
This implements #26549 (comment) for
PLSRegression. Please let me know if the alternative (#26549 (comment)) is more desired, or if I should implement this for other multi-output regressors as well in this PR.