[go: nahoru, domu]

Skip to content
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

FIX Fixes HTML repr when get_params contains classes #24512

Merged
merged 3 commits into from
Sep 30, 2022

Conversation

thomasjpfan
Copy link
Member
@thomasjpfan thomasjpfan commented Sep 26, 2022

Reference Issues/PRs

Fixes issue seen in #22856 (comment)
Fixes #24545 (comment)

What does this implement/fix? Explain your changes.

The underlying issue comes from get_params returning a estimator class as a key and not instances. This PR skips classes in the repr.

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. If you can link the other issue.
I still think that those estimators will not be compatible with GridSearchCV since get_params would not work on a class object.

Copy link
Member
@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Thanks @thomasjpfan .

This is related to #24545, but I think the fix to that issue would be #24545 (comment)

Copy link
Member
@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM.

@ogrisel
Copy link
Member
ogrisel commented Sep 30, 2022

I think this is already a net improvement. Merging.

@ogrisel ogrisel merged commit b3b2314 into scikit-learn:main Sep 30, 2022
thomasjpfan added a commit to thomasjpfan/scikit-learn that referenced this pull request Oct 9, 2022
)



Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Oct 31, 2022
)



Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when returning embedded transformers in Jupyter notebook
4 participants