ENH Adds n_features_in_ to ensemble module#19326
ENH Adds n_features_in_ to ensemble module#19326glemaitre merged 20 commits intoscikit-learn:mainfrom
Conversation
|
Can someone help me out with the CI error? |
thomasjpfan
left a comment
There was a problem hiding this comment.
Minor comment, otherwise LGTM!
| def _check_X(self, X): | ||
| return check_array(X, accept_sparse=['csr', 'csc'], ensure_2d=True, | ||
| allow_nd=True, dtype=None) | ||
| return self._validate_data( |
There was a problem hiding this comment.
I had to double check that _check_X was only called during non-fit methods. I think leaving a comment here would be best for now.
def _check_X(self, X):
# Only called to validate `X` in non-`fit` methods.
glemaitre
left a comment
There was a problem hiding this comment.
LGTM apart of the single nitpick
| Transformed dataset. | ||
| """ | ||
| X = check_array(X, accept_sparse=['csc']) | ||
| X = self._validate_data(X, accept_sparse=['csc']) |
There was a problem hiding this comment.
Uhm what is the reason that the common test where not failing for this transformer since we did not introduce _validate_data before
There was a problem hiding this comment.
test_check_n_features_in_after_fitting is applied to all estimators except those from modules listed in N_FEATURES_IN_AFTER_FIT_MODULES_TO_IGNORE. Every module where we add n_features_in_ has to be removed from that list. This is done in this PR form ensemble.
Or do you think of another test?
There was a problem hiding this comment.
I was thinking about check_n_features_in(name, estimator_orig) but I can check on the side.
Sorry we merged a new PR that added some conflicts in test_bagging again.
|
@glemaitre Thanks for merging main. I had not seen merge conflicts here on github. |
Reference Issues/PRs
Continues #18514 and #19333.