Skip to content
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

[MRG+2] addresses #8509 improvements to f_regression documentation #8548

Merged

Conversation

brownsarahm
Copy link
Contributor

@brownsarahm brownsarahm commented Mar 6, 2017

Clarified the role of this function in documentation as requested and added additional see also for more complete context.

@raghavrv
Copy link
Member

raghavrv commented Mar 6, 2017

Tests are failing due to a flake8 issue... Could you fix that... Otherwise LGTM... 2nd review by @lesteve? or @TomDLT or @NelleV :)

@raghavrv raghavrv changed the title [MRG] addresses #8509 improvements to f_regression documentation [MRG + 1] addresses #8509 improvements to f_regression documentation Mar 6, 2017
Sarah Brown and others added 4 commits March 6, 2017 16:15
@NelleV
Copy link
Member

NelleV commented Mar 7, 2017

Thanks ! It looks much better to me!
I think I fixed the last pep8 error via github's UI interface (I thought it created a PR instead of committing directly in your branch…)

@NelleV NelleV changed the title [MRG + 1] addresses #8509 improvements to f_regression documentation [MRG+2] addresses #8509 improvements to f_regression documentation Mar 7, 2017
that is, ((X[:, i] - mean(X[:, i])) * (y - mean_y)) / (std(X[:, i]) *
std(y)).
2. It is converted to an F score then to a p-value.

Read more in the :ref:`User Guide <univariate_feature_selection>`.
For more on usage see the :ref:`User Guide <univariate_feature_selection>`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this change. Nearly every class/function docstring includes "Read more in the".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to make it more clear what the user guide provides in relation to the content in the docstring. "Read more" left me to expect more information about f regression, but it actually is a page on feature selection. I didn't notice that the specific "read more in" was used everywhere, consistency might be worth keeping.

@jnothman
Copy link
Member

jnothman commented Mar 7, 2017 via email

@jnothman jnothman merged commit 5210f81 into scikit-learn:master Mar 7, 2017
@jnothman
Copy link
Member

jnothman commented Mar 7, 2017

Thanks!

@Przemo10 Przemo10 mentioned this pull request Mar 17, 2017
herilalaina pushed a commit to herilalaina/scikit-learn that referenced this pull request Mar 26, 2017
…mentation (scikit-learn#8548)

* clarify role of the function and streamline introduction

* added feature selection methods to see also

* completed see also

* fixed pep related formatting for flake8checks.

* fixed extra whitespace flake8 problems, remaining failure is a copied see all line from another function, the line is over by a period, does not make sense to newline that.

* one more whitespace

* FIX small pep8 error.
massich pushed a commit to massich/scikit-learn that referenced this pull request Apr 26, 2017
…mentation (scikit-learn#8548)

* clarify role of the function and streamline introduction

* added feature selection methods to see also

* completed see also

* fixed pep related formatting for flake8checks.

* fixed extra whitespace flake8 problems, remaining failure is a copied see all line from another function, the line is over by a period, does not make sense to newline that.

* one more whitespace

* FIX small pep8 error.
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
…mentation (scikit-learn#8548)

* clarify role of the function and streamline introduction

* added feature selection methods to see also

* completed see also

* fixed pep related formatting for flake8checks.

* fixed extra whitespace flake8 problems, remaining failure is a copied see all line from another function, the line is over by a period, does not make sense to newline that.

* one more whitespace

* FIX small pep8 error.
NelleV pushed a commit to NelleV/scikit-learn that referenced this pull request Aug 11, 2017
…mentation (scikit-learn#8548)

* clarify role of the function and streamline introduction

* added feature selection methods to see also

* completed see also

* fixed pep related formatting for flake8checks.

* fixed extra whitespace flake8 problems, remaining failure is a copied see all line from another function, the line is over by a period, does not make sense to newline that.

* one more whitespace

* FIX small pep8 error.
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
…mentation (scikit-learn#8548)

* clarify role of the function and streamline introduction

* added feature selection methods to see also

* completed see also

* fixed pep related formatting for flake8checks.

* fixed extra whitespace flake8 problems, remaining failure is a copied see all line from another function, the line is over by a period, does not make sense to newline that.

* one more whitespace

* FIX small pep8 error.
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
…mentation (scikit-learn#8548)

* clarify role of the function and streamline introduction

* added feature selection methods to see also

* completed see also

* fixed pep related formatting for flake8checks.

* fixed extra whitespace flake8 problems, remaining failure is a copied see all line from another function, the line is over by a period, does not make sense to newline that.

* one more whitespace

* FIX small pep8 error.
jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017
…mentation (scikit-learn#8548)

* clarify role of the function and streamline introduction

* added feature selection methods to see also

* completed see also

* fixed pep related formatting for flake8checks.

* fixed extra whitespace flake8 problems, remaining failure is a copied see all line from another function, the line is over by a period, does not make sense to newline that.

* one more whitespace

* FIX small pep8 error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants