Added user guide documentation for permutation_test_score by aditi9783 · Pull Request #14757 · scikit-learn/scikit-learn

aditi9783 · 2019-08-24T16:15:35Z

Reference Issues/PRs

Fixes #10905
Fixes #10931. Worked on the branch https://github.com/maskani-moh/scikit-learn/tree/fix-10905.

Closes #14769

What does this implement/fix? Explain your changes.

Added user guide documentation for permutation_test_score.

Any other comments?

Worked with @vmanisha at the NYC scikit-learn sprint 2019.

doc/modules/cross_validation.rst

NicolasHug · 2019-08-24T18:33:23Z

@aditi9783 should we close this PR in favor of #14769?

aditi9783 · 2019-08-24T18:39:13Z

@NicolasHug Sure.

…ction "Obtaining predictions by cross-validation" and modified function help text to follow PEP8.

NicolasHug · 2019-08-24T19:37:29Z

Actually re-opening so that @aditi9783 can update (will probably close #14769 then )

…-14757

NicolasHug

Thanks @aditi9783 and @vmanisha. I merged with master for the docs to compile (not sure what went wrong), and also made some very minor modifications.

Looks good!

NicolasHug · 2019-08-26T15:18:13Z

@rth @amueller @thomasjpfan ;)

jnothman · 2019-08-28T20:52:11Z

doc/modules/cross_validation.rst

+It also returns cross validation scores for each permutation of y labels. It
+permutes the labels of the samples and computes the p-value against the null
+hypothesis that the features and the labels are independent, meaning that there
+is no difference between the classes.


"classes" implies classification, which we aren't necessarily talking about here.

The function permutes class labels. Isn't classification implied in that case?

No, it peemutes target values for each sample, not class labels

jnothman · 2019-08-28T20:53:33Z

doc/modules/cross_validation.rst

+
+It also returns cross validation scores for each permutation of y labels. It
+permutes the labels of the samples and computes the p-value against the null
+hypothesis that the features and the labels are independent, meaning that there


Should "features" be "predictions"? Or is this right, if we add "conditioned on the estimator"?

Features are the input vectors (X), and thus not predictions.

Yeah, so you need to add "given the estimator"

jnothman · 2019-08-28T20:54:16Z

doc/modules/cross_validation.rst

+approximates the probability that the average cross-validation score would be
+obtained by chance if the target is independent of the data.
+
+It also returns cross validation scores for each permutation of y labels. It


Talking about return value here confuses the matter if what we want to talk about is how the P value is constructed

This documentation is for user guide. The function description and the paper cited give more details about how the p-value is constructed.

Ordinarily our user guide is less focused on API things like what the function returns. Conversely, a description of how the algorithm works belongs in the user guide not in the docstring.

Hi, we're trying to get all of the PR's from the WiMLDS sprint wrapped up, are we waiting on @aditi9783, or approval from another reviewer? Thanks!

Not completely sure what's going on with this one @reshamas

@kellycarmody We are waiting on @aditi9783

@aditi9783 We use the user guide to explain the math with the details necessary to explain the function. The function's docstring is usually brief and links to the user guide. In this case, moving most of the docstring into the user guide would be good.

Hi @thomasjpfan, @reshamas asked me to take over this PR, but I see that Nicolas Hug already approved the changes, and it is just waiting for one more reviewer.

Is the PR good? Or should I move most of the docstring into the user guide?

aditi9783 · 2019-09-11T05:12:36Z

This PR is completed from our (me and my sprint partner, Manisha Verma) side and is ready for merge. I don't know if we need approval from a core contributor or if there is anything else we need to do. Thanks, Aditi

…

On Wed, Sep 11, 2019, 8:48 AM kellycarmody ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In doc/modules/cross_validation.rst <#14757 (comment)> : > @@ -300,6 +300,23 @@ section. * :ref:`sphx_glr_auto_examples_model_selection_plot_cv_predict.py`, * :ref:`sphx_glr_auto_examples_model_selection_plot_nested_cross_validation_iris.py`. + +.. _cv_significance_evaluation: + +Cross-validation significance evaluation +---------------------------------------- + +Significance of cross validation scores can be evaluated using the +:func:`permutation_test_score` function. The function returns a p-value, which +approximates the probability that the average cross-validation score would be +obtained by chance if the target is independent of the data. + +It also returns cross validation scores for each permutation of y labels. It Hi, we're trying to get all of the PR's from the WiMLDS sprint wrapped up, are we waiting on @aditi9783 <https://github.com/aditi9783>, or approval from another reviewer? Thanks Not completely sure what's going on with this one @reshamas <https://github.com/reshamas> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#14757?email_source=notifications&email_token=AALMXJGABNVXXGXNP6WDO4DQJBPPJA5CNFSM4IPGTBXKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCEKC74A#discussion_r323043617>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALMXJH5X6PT64OJFQK7V7TQJBPPJANCNFSM4IPGTBXA> .

reshamas · 2019-09-11T10:25:51Z

@adijo @vmanisha
I am wondering if you need to edit the title to include "[MRG]"?

cc: @kellycarmody @NicolasHug

kellycarmody · 2019-09-18T18:17:10Z

Hi @aditi9783, Thomas Fan mentioned that they are still waiting for you to close this out. Would you be able to work on this sometime soon so we can start closing the PR's from the sprint? Thanks!

reshamas · 2019-12-26T20:15:41Z

@amueller @jnothman @thomasjpfan
This article is locked: https://ieeexplore.ieee.org/document/5360332
It looks like it is for corporate members, IEEE members or by paid subscription.

Is this typical to include a reference link in documentation that may require payment? I was attempting to access the paper to better understand the comments on this PR.

cc: @cmarmo (this PR is from NYC wimlds Aug 2019 sprint)

glemaitre · 2019-12-26T20:41:25Z

In this specific case, it is better to cite the following: http://www.jmlr.org/papers/volume11/ojala10a/ojala10a.pdf which is an open-access JMLR paper.

It should the extended version of the IEEE version which was a conference paper.

Is this typical to include a reference link in documentation that may require payment?

If we really want to acknowledge the original method of an author, it might be the only way sometimes.

reshamas · 2020-01-03T14:49:00Z

I propose adding this PR to the upcoming Berlin beginner sprint list of issues.

cc: @adrinjalali @cmarmo

adrinjalali · 2020-01-03T15:08:32Z

@reshamas do you have a repo as you usually do for the sprints to add these to a project on that repo?

reshamas · 2020-01-03T15:13:40Z

@adrinjalali
Yes, I just added you to the repo:
https://github.com/WiMLDS/berlin-2020-scikit-sprint

reshamas · 2020-01-03T15:14:46Z

also added @cmarmo to repo:
https://github.com/WiMLDS/berlin-2020-scikit-sprint

stareh · 2020-01-25T10:27:38Z

Hey I just started working on this citation problem @adrinjalali @reshamas @aditi9783
#wimlds

reshamas · 2020-06-26T00:43:52Z

@cmarmo this PR is one of the last ones from the NYC Aug 2019 sprint. Does it need a reviewer?

cmarmo · 2020-06-26T07:50:32Z

@cmarmo this PR is one of the last ones from the NYC Aug 2019 sprint. Does it need a reviewer?

Hi @reshamas , if I understand correctly, @lucyleeow has taken over in #17373.

adrinjalali · 2020-06-26T10:41:30Z

doesn't that mean this is superseded and not stalled @cmarmo ?

cmarmo · 2020-07-31T09:28:50Z

Closed by #17373.

maskani-moh and others added 4 commits April 6, 2018 13:02

Improve docstring

fe91aaf

Change whats_new

d839cbb

Fix pep8

71bdd72

Added documentation for permutation_test_score in the user guide.

573d891

amueller reviewed Aug 24, 2019

View reviewed changes

doc/modules/cross_validation.rst Outdated Show resolved Hide resolved

Wrapped user guide text to 79 char

096c3be

amueller reviewed Aug 24, 2019

View reviewed changes

doc/modules/cross_validation.rst Outdated Show resolved Hide resolved

amueller reviewed Aug 24, 2019

View reviewed changes

doc/modules/cross_validation.rst Outdated Show resolved Hide resolved

NicolasHug closed this Aug 24, 2019

Moved user guide documentation for permutation_test_score to after se…

201dc02

…ction "Obtaining predictions by cross-validation" and modified function help text to follow PEP8.

NicolasHug reopened this Aug 24, 2019

Fixed typos in the permutation_test_score documentation in user guide.

5cf5ae7

aditi9783 mentioned this pull request Aug 24, 2019

Added user guide documentation for permutation_test_score #10905 #14769

Closed

NicolasHug added 2 commits August 26, 2019 10:55

Merge branch 'master' of github.com:scikit-learn/scikit-learn into pr…

cf52380

…-14757

Minor changes

5252b82

NicolasHug approved these changes Aug 26, 2019

View reviewed changes

rth changed the title ~~Added user guide documentation for permutation_test_score #10905~~ Added user guide documentation for permutation_test_score Aug 26, 2019

jnothman reviewed Aug 28, 2019

View reviewed changes

glemaitre self-requested a review September 24, 2019 13:16

stareh mentioned this pull request Jan 25, 2020

DOC added the link for the reference in permutation_test_score #16202

Merged

github-actions bot added the module:model_selection label Mar 2, 2020

lucyleeow mentioned this pull request May 28, 2020

DOC Add doc for permutation_test_score #17373

Merged

cmarmo added the Stalled label Jun 26, 2020

cmarmo added Superseded PR has been replace by a newer PR and removed Stalled labels Jun 26, 2020

cmarmo closed this Jul 31, 2020

Uh oh!

Conversation

aditi9783 commented Aug 24, 2019 • edited by NicolasHug Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NicolasHug commented Aug 24, 2019

Uh oh!

aditi9783 commented Aug 24, 2019

Uh oh!

NicolasHug commented Aug 24, 2019

Uh oh!

NicolasHug left a comment

Choose a reason for hiding this comment

Uh oh!

NicolasHug commented Aug 26, 2019

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!

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!

kellycarmody Sep 11, 2019 • 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!

aditi9783 commented Sep 11, 2019 via email

Uh oh!

reshamas commented Sep 11, 2019

Uh oh!

kellycarmody commented Sep 18, 2019

Uh oh!

reshamas commented Dec 26, 2019

Uh oh!

glemaitre commented Dec 26, 2019

Uh oh!

reshamas commented Jan 3, 2020

Uh oh!

adrinjalali commented Jan 3, 2020

Uh oh!

reshamas commented Jan 3, 2020

Uh oh!

reshamas commented Jan 3, 2020

Uh oh!

stareh commented Jan 25, 2020

Uh oh!

reshamas commented Jun 26, 2020

Uh oh!

cmarmo commented Jun 26, 2020

Uh oh!

adrinjalali commented Jun 26, 2020

Uh oh!

cmarmo commented Jul 31, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

aditi9783 commented Aug 24, 2019 •

edited by NicolasHug

Loading

kellycarmody Sep 11, 2019 •

edited

Loading