-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
MNT move PolynomialFeatures from _data.py to _polynomial.py #19611
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
Conversation
Is changelog entry required here? |
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.
Could you also deal with test_polynomial_and_spline_array_order
and its comment?
sklearn/preprocessing/__init__.py
Outdated
from ._polynomial import SplineTransformer | ||
from ._polynomial import PolynomialFeatures |
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.
Change the order`
sklearn/preprocessing/_polynomial.py
Outdated
@@ -427,3 +432,290 @@ def transform(self, X): | |||
j for j in range(XBS.shape[1]) if (j + 1) % n_splines != 0 | |||
] | |||
return XBS[:, indices] | |||
|
|||
|
|||
class PolynomialFeatures(TransformerMixin, BaseEstimator): |
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.
I would place it before the splines.
from sklearn.preprocessing import KBinsDiscretizer, SplineTransformer | ||
from sklearn.preprocessing import PolynomialFeatures |
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.
from sklearn.preprocessing import KBinsDiscretizer, SplineTransformer | |
from sklearn.preprocessing import PolynomialFeatures | |
from sklearn.preprocessing import ( | |
KBinsDiscretizer, PolynomialFeatures, SplineTransformer | |
) |
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.
LGTM
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.
Thanks!
Reference Issues/PRs
Fixes #19574
What does this implement/fix? Explain your changes.
I moved class PolynomialFeatures from _data.py to _polynomial.py as
mentioned in the issue. I also moved all tests relevant to PolynomialFeatures
from tests_data.py to tests_polynomial.py
Any other comments?
Please mention if I left something.