API Deprecates the fit_grid_point function by ariepratama · Pull Request #16401 · scikit-learn/scikit-learn

ariepratama · 2020-02-06T14:19:54Z

Reference Issues/PRs

Fixes #16391

What does this implement/fix? Explain your changes.

Add deprecation notice to fit_grid_point function because it is not used anywhere except test

Any other comments?

thomasjpfan

This needs a test to make sure the deprecation is raised with the expected message.

ogrisel

LGTM.

ogrisel

Actually not completely good:

the fit_grid_point is deprecated in version 0.23 and will be removed in 0.25
there is a training whitespace that breaks the linter: https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=13337&view=logs&j=32e2e1bb-a28f-5b18-6cfc-3f01273f5609&t=f370d024-dfa6-536d-2b2d-766ea2a2900c

0.22 was already released. The next release will be 0.23.

ariepratama · 2020-02-09T14:23:31Z

the fit_grid_point is deprecated in version 0.23 and will be removed in 0.25
done

there is a training whitespace that breaks the linter: https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=13337&view=logs&j=32e2e1bb-a28f-5b18-6cfc-3f01273f5609&t=f370d024-dfa6-536d-2b2d-766ea2a2900c
done

thomasjpfan

Add a comment above test_fit_grid_point to remove it in 0.25.

We will squash your PR when it is merge. There is no need to push force push your changes.

thomasjpfan · 2020-02-09T23:39:54Z

+    with pytest.warns(FutureWarning, match=msg):
+        for params in ({'C': 0.1}, {'C': 0.01}, {'C': 0.001}):
+            for train, test in cv.split(X, y):
+                this_scores, this_params, n_test_samples = fit_grid_point(


Move the pytest.warns here to be specific where the warning comes from.

with pytest.warns(...): this_scores, this_params, =

It will also raise deprecation warning on this code

assert_raise_message(ValueError, "For evaluating multiple scores, use " "sklearn.model_selection.cross_validate instead.", fit_grid_point, X, y, svc, params, train, test, {'score': scorer}, verbose=True)

and make the azure test pipeline failed

should I applied pytest.warns(FutureWarning, match=msg) twice? (1 inside training loop and another before assert_raise_message)

Lets not change this test and ignore the FutureWarning:

# FIXME Remove when fit_grid_point is deprecated in 0.25 @ignore_warnings(category=FutureWarning) def test_fit_grid_point(): ...

And add another tests that only tests the deprecation:

# FIXME Remove when fit_grid_point is deprecated in 0.25 def test_fit_grid_point_deprecated(): with pytest.warns(FutureWarning, ...): ...

…recation warning

thomasjpfan

Please add an entry to the change log at doc/whats_new/v0.23.rst with tag API. Like the other entries there, please reference this pull request with :pr: and credit yourself (and other contributors if applicable) with :user:.

Also update doc/modules/classes.rst and move model_selection.fit_grid_point to the deprecated section.

thomasjpfan · 2020-02-11T17:10:41Z

+        "fit_grid_point is deprecated in version 0.23 "
+        "and will be removed in version 0.25")
+    params = {'C': 0.1}
+    train, test = next(cv.split(X, y))


Nit:

Suggested change

train, test = next(cv.split(X, y))

train, test = next(StratifiedKFold().split(X, y))

and remove the cv definition above.

thomasjpfan · 2020-02-11T17:11:19Z

+    train, test = next(cv.split(X, y))
+
+    with pytest.warns(FutureWarning, match=msg):
+        fit_grid_point(


Nit: (fits in one line < 80)

Suggested change

fit_grid_point(

fit_grid_point(X, y, svc, params, train, test, scorer, verbose=False)

thomasjpfan · 2020-02-11T17:11:49Z

+    cv = StratifiedKFold()
+    svc = LinearSVC(random_state=0)
+    scorer = make_scorer(accuracy_score)
+    msg = (


Nit: No new line needed

Suggested change

msg = (

msg = ("fit_grid_point is deprecated in version 0.23 "

"and will be removed in version 0.25")

thomasjpfan · 2020-02-13T16:00:37Z

+
+.. autosummary::
+   :toctree: generated/
+   :template: deprecated_function.rst


.. autosummary:: :toctree: generated/ :template: deprecated_function.rst model_selection.fit_grid_point utils.safe_indexing

thomasjpfan · 2020-02-13T16:00:58Z

+    svc = LinearSVC(random_state=0)
+    scorer = make_scorer(accuracy_score)
+    msg = ("fit_grid_point is deprecated in version 0.23 "
+            "and will be removed in version 0.25")


Linting:

Suggested change

"and will be removed in version 0.25")

"and will be removed in version 0.25")

thomasjpfan · 2020-02-14T03:07:20Z

Thank you @ariepratama !

ariepratama · 2020-02-14T06:32:40Z

sorry to give you headache when reviewing my codes @thomasjpfan

ariepratama added 2 commits February 6, 2020 21:15

FIX add deprecation to function fit_grid_point

4db5190

FIX add 1 line between function fit_grid_point

8e62d57

thomasjpfan reviewed Feb 7, 2020

View reviewed changes

Comment thread sklearn/model_selection/_search.py Outdated

ariepratama added 2 commits February 8, 2020 13:56

FIX change deprecation message simpler and test deprecation warning

2eb7876

FIX delete extra new line

eab79e5

ogrisel approved these changes Feb 8, 2020

View reviewed changes

ogrisel reviewed Feb 8, 2020

View reviewed changes

ariepratama added 2 commits February 8, 2020 18:13

FIX message deprecation to 0.22 -> 0.23

6d6a994

FIX expect deprecation warning for all fit_grid_point

6ca98bd

thomasjpfan reviewed Feb 9, 2020

View reviewed changes

ariepratama added 2 commits February 10, 2020 22:07

FIX ensure FutureWarning to be specific call on fit_grid_point

f38e675

FIX separate testing functionality of fit_grid_point and throwing dep…

3e10078

…recation warning

thomasjpfan reviewed Feb 11, 2020

View reviewed changes

FIX applying suggestion on code formats and document changes

b92c21c

thomasjpfan reviewed Feb 13, 2020

View reviewed changes

ariepratama and others added 5 commits February 14, 2020 06:23

FIX remove extra space

cc0aca9

FIX remove code duplication in documentation

d96369d

Merge remote-tracking branch 'upstream/master' into pr/16401

2ab3109

CLN Cleans up PR

ca3989d

DOC Fixes pr rst

3a31578

thomasjpfan approved these changes Feb 14, 2020

View reviewed changes

thomasjpfan changed the title ~~FIX add deprecation to function fit_grid_point~~ API Deprecates the fit_grid_point function Feb 14, 2020

thomasjpfan merged commit c2e742c into scikit-learn:master Feb 14, 2020

thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 22, 2020

API Deprecates the fit_grid_point function (scikit-learn#16401)

0489df1

panpiort8 pushed a commit to panpiort8/scikit-learn that referenced this pull request Mar 3, 2020

API Deprecates the fit_grid_point function (scikit-learn#16401)

eb51216

	train, test = next(cv.split(X, y))
	train, test = next(StratifiedKFold().split(X, y))

	fit_grid_point(
	fit_grid_point(X, y, svc, params, train, test, scorer, verbose=False)

	msg = (
	msg = ("fit_grid_point is deprecated in version 0.23 "
	"and will be removed in version 0.25")

Uh oh!

Uh oh!

Conversation

ariepratama commented Feb 6, 2020

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

thomasjpfan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

ogrisel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ariepratama commented Feb 9, 2020

Uh oh!

thomasjpfan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ariepratama Feb 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomasjpfan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomasjpfan commented Feb 14, 2020

Uh oh!

ariepratama commented Feb 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ogrisel left a comment •

edited

Loading

ariepratama Feb 10, 2020 •

edited

Loading