DOC: Clarify `cv` parameter description in `GridSearchCV` by QBatista · Pull Request #12495 · scikit-learn/scikit-learn

QBatista · 2018-10-31T01:11:30Z

Reference Issues/PRs

This PR addresses issue #12466.

What does this implement/fix? Explain your changes.

This PR does the 3 following things:

Rewrite the cv parameter description in GridSearchCV
Link the new CV splitter description to an existing example
Add an example with a custom iterable

Thanks for reviewing this!

- Rewrite the parameter description - Link the `CV splitter` description to an existing example - Add an example with a custom iterable

jnothman

This is relevant to all other *CV and cross validation functions... This pull request should change many docstrings if it changes one

jnothman · 2018-10-31T22:09:59Z

sklearn/model_selection/_search.py

        - integer, to specify the number of folds in a `(Stratified)KFold`,
-        - An object to be used as a cross-validation generator.
-        - An iterable yielding train, test splits.
+        - A :term:`CV splitter <CV splitter>` object. See


:term:`CV splitter` should be sufficient

jnothman · 2018-10-31T22:10:35Z

sklearn/model_selection/_search.py

+        - A :term:`CV splitter <CV splitter>` object. See
+          :ref:`sphx_glr_auto_examples_svm_plot_rbf_parameters.py` for
+          an example.
+        - An iterable yielding train, test splits as arrays of indices.


(train, test)

jnothman · 2018-10-31T22:12:53Z

sklearn/model_selection/_search.py

     'split2_test_score', ...
     'std_fit_time', 'std_score_time', 'std_test_score']

+    >>> from sklearn import svm, datasets


This rather belongs in doc/modules/grid_search.rst or
../cross_validation.rst or doc/glossary.rst if not covered in those already

…ption the `cv` parameter.

…ion functions.

QBatista · 2018-11-03T12:52:52Z

@jnothman Thanks for your comments! I've made changes to the following CV and cross validation functions in addition to GridSearchCV:

check_cv
RandomizedSearchCV
cross_validate
cross_val_predict
cross_val_score
learning_curve
permutation_test_score
validation_curve

There are other places where the cv parameter is used such as ClassifierChain. Should I update them as well? Also, there seems to be a redundancy in the glossary: there is both CV splitter and cross-validation splitter. Is this intended or should one of them be removed?

jnothman · 2018-11-05T22:12:29Z

There aren't two entries are there, but two terms for one entry? Yes, that is intended.

jnothman

git grep -p "An object to be used as a" shows many more places where you should make this change.

jnothman · 2018-11-08T23:03:10Z

sklearn/calibration.py

        - integer, to specify the number of folds.
-        - An object to be used as a cross-validation generator.
-        - An iterable yielding train/test splits.
+        - :term:`CV splitter <CV splitter>`,


You still shouldn't have that expression in angle brackets. It is unnecessary when the text to display is the same as the link target name

QBatista · 2018-11-09T09:29:11Z

@jnothman Thanks for letting me know! Hopefully, everything is now alright.

…ybutton * upstream/master: DOC: Clarify `cv` parameter description in `GridSearchCV` (scikit-learn#12495)

…rn#12495) #### Reference Issues/PRs  This PR addresses issue scikit-learn#12466. #### What does this implement/fix? Explain your changes. This PR does the 3 following things: - Rewrite the `cv` parameter description in `GridSearchCV` - Link the new `CV splitter` description to an existing example - Add an example with a custom iterable Thanks for reviewing this! Close scikit-learn#12466

…ikit-learn into add_codeblock_copybutton * 'add_codeblock_copybutton' of https://github.com/thoo/scikit-learn: Move an extension under sphinx_copybutton/ Move css/js file under sphinxext/ Fix max_depth overshoot in BFS expansion of trees (scikit-learn#12344) TST don't test utils.fixes docstrings (scikit-learn#12576) DOC Fix typo (scikit-learn#12563) FIX Workaround limitation of cloudpickle under PyPy (scikit-learn#12566) MNT bare asserts (scikit-learn#12571) FIX incorrect error when OneHotEncoder.transform called prior to fit (scikit-learn#12443) Retrigger travis:max time limit error DOC: Clarify `cv` parameter description in `GridSearchCV` (scikit-learn#12495) FIX remove FutureWarning in _object_dtype_isnan and add test (scikit-learn#12567) DOC Add 's' to "correspond" in docs for Hamming Loss. (scikit-learn#12565) EXA Fix comment in plot-iris-logistic example (scikit-learn#12564) FIX stop words validation in text vectorizers with custom preprocessors / tokenizers (scikit-learn#12393) DOC Add skorch to related projects (scikit-learn#12561) MNT Don't change self.n_values in OneHotEncoder.fit (scikit-learn#12286) MNT Remove unused assert_true imports (scikit-learn#12560) TST autoreplace assert_true(...==...) with plain assert (scikit-learn#12547) DOC: add a testimonial from JP Morgan (scikit-learn#12555)

…rn#12495) #### Reference Issues/PRs  This PR addresses issue scikit-learn#12466. #### What does this implement/fix? Explain your changes. This PR does the 3 following things: - Rewrite the `cv` parameter description in `GridSearchCV` - Link the new `CV splitter` description to an existing example - Add an example with a custom iterable Thanks for reviewing this! Close scikit-learn#12466

…ikit-learn#12495)" This reverts commit 6878a70.

…rn#12495) #### Reference Issues/PRs  This PR addresses issue scikit-learn#12466. #### What does this implement/fix? Explain your changes. This PR does the 3 following things: - Rewrite the `cv` parameter description in `GridSearchCV` - Link the new `CV splitter` description to an existing example - Add an example with a custom iterable Thanks for reviewing this! Close scikit-learn#12466

QBatista added 2 commits October 31, 2018 05:01

DOC: Clarify cv parameter description in GridSearchCV

92d76e8

- Rewrite the parameter description - Link the `CV splitter` description to an existing example - Add an example with a custom iterable

Use ellipsis for the doc test to pass

e2d787f

jnothman reviewed Oct 31, 2018

View reviewed changes

QBatista added 2 commits November 3, 2018 16:40

Move the new example to cross_validation.rst and shorten the descri…

7edd593

…ption the `cv` parameter.

Rewrite the cv parameter description for other CV and cross validat…

482a4df

…ion functions.

Rename variable to avoid overwriting

f84279b

jnothman reviewed Nov 5, 2018

View reviewed changes

Rewrite remaining cv parameter descriptions

e2ffd55

jnothman reviewed Nov 8, 2018

View reviewed changes

Change ":term:CV splitter <CV splitter>" to ":term:CV splitter"

e317416

jnothman approved these changes Nov 10, 2018

View reviewed changes

eamanu approved these changes Nov 11, 2018

View reviewed changes

amueller approved these changes Nov 12, 2018

View reviewed changes

amueller merged commit 4b78d7a into scikit-learn:master Nov 12, 2018

thoo added a commit to thoo/scikit-learn that referenced this pull request Nov 12, 2018

Merge remote-tracking branch 'upstream/master' into add_codeblock_cop…

2326f8a

…ybutton * upstream/master: DOC: Clarify `cv` parameter description in `GridSearchCV` (scikit-learn#12495)

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

Revert "DOC: Clarify cv parameter description in GridSearchCV (sc…

b22ae86

…ikit-learn#12495)" This reverts commit 6878a70.

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

Revert "DOC: Clarify cv parameter description in GridSearchCV (sc…

c03dfac

…ikit-learn#12495)" This reverts commit 6878a70.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

DOC: Clarify `cv` parameter description in `GridSearchCV`#12495

DOC: Clarify `cv` parameter description in `GridSearchCV`#12495
amueller merged 7 commits intoscikit-learn:masterfrom
QBatista:fix_doc_cv

QBatista commented Oct 31, 2018

Uh oh!

jnothman left a comment

Uh oh!

jnothman Oct 31, 2018

Uh oh!

jnothman Oct 31, 2018

Uh oh!

jnothman Oct 31, 2018

Uh oh!

QBatista commented Nov 3, 2018

Uh oh!

jnothman commented Nov 5, 2018 via email

Uh oh!

jnothman left a comment

Uh oh!

jnothman Nov 8, 2018

Uh oh!

QBatista commented Nov 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

QBatista commented Oct 31, 2018

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

jnothman Oct 31, 2018

Choose a reason for hiding this comment

Uh oh!

jnothman Oct 31, 2018

Choose a reason for hiding this comment

Uh oh!

jnothman Oct 31, 2018

Choose a reason for hiding this comment

Uh oh!

QBatista commented Nov 3, 2018

Uh oh!

jnothman commented Nov 5, 2018 via email

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

jnothman Nov 8, 2018

Choose a reason for hiding this comment

Uh oh!

QBatista commented Nov 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants