-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
MRG add n_features_out_ attribute #14241
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
Changes from all commits
c47227d
06b4a08
b13b57e
ac8d243
822dae6
211ebd5
2933b8b
42e5017
124e325
041dfff
c1d47a1
7735d18
aef2283
6a56572
5c267ce
9a2e80c
375c130
0c2bb8a
9ccf0ac
186a6d2
6043a5c
a39ab07
8039d8e
344d01e
df82f64
816b677
dcba760
d3b02e4
e02d118
c8db102
4b539af
43e9c96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
import copy | ||
import warnings | ||
from collections import defaultdict | ||
import numbers | ||
import platform | ||
import inspect | ||
import re | ||
|
@@ -555,6 +556,33 @@ def fit_transform(self, X, y=None, **fit_params): | |
# fit method of arity 2 (supervised transformation) | ||
return self.fit(X, y, **fit_params).transform(X) | ||
|
||
@property | ||
def n_features_out_(self): | ||
return self._n_features_out | ||
|
||
@n_features_out_.setter | ||
def n_features_out_(self, val): | ||
self._n_features_out = val | ||
|
||
|
||
class ComponentsMixin: | ||
@property | ||
def n_features_out_(self): | ||
if hasattr(self, 'n_components_'): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we consider deprecating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would consider it ;) |
||
# n_components could be auto or None | ||
# this is more likely to be an int | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can also include the |
||
n_features = self.n_components_ | ||
elif hasattr(self, 'components_'): | ||
n_features = self.components_.shape[0] | ||
elif (hasattr(self, 'n_components') | ||
and isinstance(self.n_components, numbers.Integral)): | ||
n_features = self.n_components | ||
else: | ||
raise AttributeError( | ||
"{} has no attribute 'n_features_out_'".format( | ||
type(self).__name__)) | ||
return n_features | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should there be a "default" value here? Maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here? This is for In general: the route I'm taking here is to require the user to set it to |
||
|
||
|
||
class DensityMixin: | ||
"""Mixin class for all density estimators in scikit-learn.""" | ||
|
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 can't say I like this magic determination. I'd rather it be done by specialised mixins for decomposition and feature selection.
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.
Sure, I could do that.
I have to check how much of these are actually in the
decomposition
moduleThere 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'm not sure if I prefer having these be mixins or base classes. It seems unlikely you want to mix those and base classes make the code shorter.