-
Notifications
You must be signed in to change notification settings - Fork 42
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
version 1.1.2 update #33
Conversation
Signed-off-by: Filip Michalsky <[email protected]>
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.
Overall looks good, just a suggestion for the changelog
April, 15, 2021 1.11.2 | ||
------------------------------------------------------------------------------- | ||
minor: | ||
- Updated dtype from np.int to int in base_mab.py line 294 to resolve deprecation warning from numpy>=1.20.0 |
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.
I like the specificity 😆
CHANGELOG.txt
Outdated
------------------------------------------------------------------------------- | ||
minor: | ||
- Updated dtype from np.int to int in base_mab.py line 294 to resolve deprecation warning from numpy>=1.20.0 | ||
- Updated tests to reflect changes in KMeans update in sklearn>=0.24.0 |
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.
The change is to the sklearn version dependency, the unit tests are incidental to that. Suggested rephrasing: "Updated sklearn version dependency to >=0.24.0 because of changes in the KMeans implementation"
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.
thank you, updated.
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.
-
How many times are they gonna re-implement K-Means from scratch.... :D
-
This might then have an impact anywhere else we use K-Means (e.g, Content Selection..)
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.
@skadio I will check all other libraries as well
Signed-off-by: Filip Michalsky <[email protected]>
@@ -1,6 +1,6 @@ | |||
joblib | |||
numpy | |||
pandas | |||
scikit-learn>=0.22.0 | |||
scikit-learn>=0.24.0 |
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.
Not about this PR, but I am very curious how >= 0.24 will play with other libraries. It would be amazing if we can make a jump to the newer version, across the board.
version 1.1.2:
Signed-off-by: Filip Michalsky [email protected]