-
-
Notifications
You must be signed in to change notification settings - Fork 25.2k
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
Conversation
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.
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.
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.
Thanks @thomasjpfan .
This is related to #24545, but I think the fix to that issue would be #24545 (comment)
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.
LGTM.
I think this is already a net improvement. Merging. |
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.