-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
DEP loss "log" in favor of "log loss" in SGDClassifier #23046
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
DEP loss "log" in favor of "log loss" in SGDClassifier #23046
Conversation
There are some tests that still pass diff --git a/sklearn/tests/test_multioutput.py b/sklearn/tests/test_multioutput.py
index ded47c4fe..17b9bd0d7 100644
--- a/sklearn/tests/test_multioutput.py
+++ b/sklearn/tests/test_multioutput.py
@@ -257,7 +257,7 @@ def test_multi_output_classification_partial_fit():
def test_multi_output_classification_partial_fit_no_first_classes_exception():
- sgd_linear_clf = SGDClassifier(loss="log", random_state=1, max_iter=5)
+ sgd_linear_clf = SGDClassifier(loss="log_loss", random_state=1, max_iter=5)
multi_target_linear = MultiOutputClassifier(sgd_linear_clf)
msg = "classes must be passed on the first call to partial_fit."
with pytest.raises(ValueError, match=msg):
Another possibility is to use the following line to find the pattern of such tests: |
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.
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 up to a few modifications.
- For :class:`linear_model.SGDClassifier`, the `loss` parameter name | ||
"log" is deprecated in favor of the new name "log_loss". | ||
:pr:`23046` by :user:`Christian Lorentzen <lorentzenchr>`. |
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.
- For :class:`linear_model.SGDClassifier`, the `loss` parameter name | |
"log" is deprecated in favor of the new name "log_loss". | |
:pr:`23046` by :user:`Christian Lorentzen <lorentzenchr>`. | |
- For :class:`linear_model.SGDClassifier`, the `loss` parameter name | |
`"log"` is deprecated in favor of the new name `"log_loss"`. | |
:pr:`23046` by :user:`Christian Lorentzen <lorentzenchr>`. |
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.
We merged #23036 without the backticks, let's keep local consistency
Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>
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
@jeremiedbb Again, thanks for finishing this. |
…23046) Co-authored-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
scikit-learn/scikit-learn/pull 82F8 /23046 Introduces `'log_loss'` in `SGDClassifier` and deprecates `'log'`. This PR addresses this issue in the repository (no other files in the repo need to be modified for this issue)
Reference Issues/PRs
Partially addresses #18248
What does this implement/fix? Explain your changes.
This PR introduces
loss="log_loss"
for SGDClassifier and deprecatesloss="log"
.Any other comments?