Skip to content

ENH Get list of estimators not raising NotFittedError #25221

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

ENH Get list of estimators not raising NotFittedError #25221

wants to merge 2 commits into from

Conversation

Kshitij68
Copy link
Contributor

Reference Issues/PRs

Generates test case for #24916
Generates a list of estimators that do not raise NotFittedError

What does this implement/fix? Explain your changes.

  • Find estimators which have get_feature_names_out attribute. The estimator list is retrieved from sklearn.utils.all_estimators
  • Initialize the estimator correctly by passing mandatory args (This is bit hacky rn, let me know if something better can be done)
  • call get_feature_names_out and assert NotFittedError

@Kshitij68 Kshitij68 mentioned this pull request Dec 22, 2022
Closed
36 tasks
@Kshitij68 Kshitij68 changed the title Get list of estimators not raising notfittederror Get list of estimators not raising NotFittedError Dec 22, 2022
@Kshitij68 Kshitij68 changed the title Get list of estimators not raising NotFittedError ENH Get list of estimators not raising NotFittedError Dec 22, 2022
@jpangas
Copy link
Contributor

jpangas commented Dec 22, 2022 •
edited
Loading

@Kshitij68 ,thank you for taking this up. there are a few changes I suggest to conform with the uniformity of other estimator_checks.

  1. We can write a function called check_estimator_get_feature_names_outin estimator_checks.py
  2. We import this function into test_common.py

@glemaitre
Copy link
Member

I already reviewed #25223
I think the code proposed there is more atomic and easier to understand.
I just proposed adding the whitelist to accept that some estimators will fail.
Then, we can start with subsequent PR to fix those estimators.

Therefore, I am closing this PR.

@glemaitre
Copy link
Member

@Kshitij68 feel free to fix any of the subsequent failing estimators.

@glemaitre glemaitre closed this Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Kshitij68 @jpangas @glemaitre