Skip to content

ENH Raise NotFittedError in get_feature_names_out for estimators that use OnetoOneFeatureMixin #25294

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

jpangas
Copy link
Contributor

@jpangas jpangas commented Jan 4, 2023

Reference Issues/PRs

In issue #24916, we want to make the error message uniform in estimators when we call get_feature_names_out before fit. This PR works towards that issue and solves the inconsistencies in estimators that inherit from the OnetoOneFeatureMixin Class.

What does this implement/fix? Explain your changes.

The following estimators that inherit from the OnetoOneFeatureMixin class will raise the agreed NotFittedError when get_feature_names_out is called before fit.

  • Binarizer
  • MaxAbsScaler
  • MinMaxScaler
  • Normalizer
  • OrdinalEncoder
  • PowerTransformer
  • QuantileTransformer
  • RobustScaler
  • StandardScaler
  • TfidfTransformer

Any other comments?

All tests successfully passed after the above estimators were removed from the whitelist.

@jpangas
Copy link
Contributor Author

jpangas commented Jan 4, 2023

@adrinjalali , I had made an error in the previous PR and closed it. I have made a new PR (this one) with the change log entry.

@adrinjalali adrinjalali added Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one! labels Jan 5, 2023
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

LGTM also. We only need to move the entry in the right log file.

@@ -180,6 +180,26 @@ Changes impacting all modules
:user:`Olivier Grisel <ogrisel>`, and `Thomas Fan`_,
:pr:`24556` by :user:`Vincent Maladière <Vincent-Maladiere>`.

- |Enhancement| The `get_feature_names_out` method of the following classes now
Copy link
Member

Choose a reason for hiding this comment

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

We need to place this entry in 1.3.rst.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Working on it.

@jpangas jpangas requested a review from glemaitre January 5, 2023 12:18
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @jpangas

@glemaitre glemaitre merged commit 35b5ee6 into scikit-learn:main Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants