Skip to content

FIX ravel prediction of PLSRegression when fitted on 1d y #26602

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

Merged
merged 8 commits into from
Jul 24, 2023
7 changes: 7 additions & 0 deletions doc/whats_new/v1.4.rst
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ Changelog
- |Enhancement| :func:`base.clone` now supports `dict` as input and creates a
copy. :pr:`26786` by `Adrin Jalali`_.

:mod:`sklearn.cross_decomposition`
..................................

- |Fix| :class:`cross_decomposition.PLSRegression` now automatically ravels the output
of `predict` if fitted with one dimensional `y`.
:pr:`26602` by :user:`Yao Xiao <Charlie-XIAO>`.

:mod:`sklearn.decomposition`
............................

Expand Down
7 changes: 5 additions & 2 deletions sklearn/cross_decomposition/_pls.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,10 @@ def fit(self, X, Y):
Y, input_name="Y", dtype=np.float64, copy=self.copy, ensure_2d=False
)
if Y.ndim == 1:
self._predict_1d = True
Y = Y.reshape(-1, 1)
else:
self._predict_1d = False

n = X.shape[0]
p = X.shape[1]
Expand Down Expand Up @@ -469,8 +472,8 @@ def predict(self, X, copy=True):
# Normalize
X -= self._x_mean
X /= self._x_std
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
Copy link
Member

Choose a reason for hiding this comment

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

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 :-/

Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion I think it is fine both ways.


def fit_transform(self, X, y=None):
"""Learn and apply the dimension reduction on the train data.
Expand Down
23 changes: 23 additions & 0 deletions sklearn/cross_decomposition/tests/test_pls.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
_svd_flip_1d,
)
from sklearn.datasets import load_linnerud, make_regression
from sklearn.ensemble import VotingRegressor
from sklearn.exceptions import ConvergenceWarning
from sklearn.linear_model import LinearRegression
from sklearn.utils import check_random_state
from sklearn.utils.extmath import svd_flip

Expand Down Expand Up @@ -621,3 +623,24 @@ def test_pls_set_output(Klass):
assert isinstance(y_trans, np.ndarray)
assert isinstance(X_trans, pd.DataFrame)
assert_array_equal(X_trans.columns, est.get_feature_names_out())


def test_pls_regression_fit_1d_y():
"""Check that when fitting with 1d `y`, prediction should also be 1d.

Non-regression test for Issue #26549.
"""
X = np.array([[1, 1], [2, 4], [3, 9], [4, 16], [5, 25], [6, 36]])
y = np.array([2, 6, 12, 20, 30, 42])
expected = y.copy()

plsr = PLSRegression().fit(X, y)
y_pred = plsr.predict(X)
assert y_pred.shape == expected.shape

# Check that it works in VotingRegressor
lr = LinearRegression().fit(X, y)
vr = VotingRegressor([("lr", lr), ("plsr", plsr)])
y_pred = vr.fit(X, y).predict(X)
assert y_pred.shape == expected.shape
assert_allclose(y_pred, expected)