-
-
Notifications
You must be signed in to change notification settings - Fork 562
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 KElbow get_params #1251
Fix KElbow get_params #1251
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1251 +/- ##
========================================
Coverage 90.57% 90.58%
========================================
Files 92 92
Lines 5209 5213 +4
========================================
+ Hits 4718 4722 +4
Misses 491 491
Continue to review full report at Codecov.
|
It was dropping curve... |
@bbengfort You should open an issue to create get_params test. |
I just checked |
I also added a test for |
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.
Looks good to me
This PR fixes #1232 which reported a bug that caused a problem to occur when users call
KElbow.get_params()
, e.g. in a notebook when they want to render the scikit-learn rich visualization of the estimator.I have made the following changes:
KElbow
to ensure it stores all hyperparams and only creates learned params infit()
get_params
works forKElbow
Sample Code and Plot
The above code snippet should not produce an exception:
AttributeError: 'KMeans' object has no attribute 'k'
Questions for the @DistrictDataLabs/team-oz-maintainers:
get_params
for all our models to make sure this doesn't happen?CHECKLIST
Have you noted the new functionality/bugfix in the release notes of the next release?Included a sample plot to visually illustrate your changes?Have you updated the baseline images if necessary?pytest
?Have you documented your new feature/functionality in the docs?Have you built the docs usingmake html
?