[MRG + 1 (rv) + 1 (alex) + 1] Add a check to test the docstring params and their order #9206

raghavrv · 2017-06-23T11:20:37Z

Taking over Alex's #9023

Fixes #7758

Also refer alternative implementation at #7793

What does this implement/fix? Explain your changes.

This adds a unit test to check that all parameters are documented and in the right order.

cc: @agramfort @neerajgangwar

Update with master and fix merge conflicts

raghavrv · 2017-06-23T11:21:29Z

These need fixing - EDIT: Fixed

E           sklearn.covariance.outlier_detection.__init__ arg mismatch: ['random_state'] (EllipticEnvelope)
E           sklearn.decomposition.fastica_.transform copy != y (FastICA)
E           sklearn.decomposition.fastica_.transform y != copy (FastICA)
E           sklearn.linear_model.coordinate_descent.enet_path arg mismatch: ['params']
E           sklearn.linear_model.coordinate_descent.enet_path arg mismatch: ['params'] (ElasticNet)
E           sklearn.linear_model.coordinate_descent.enet_path arg mismatch: ['params'] (ElasticNetCV)
E           sklearn.linear_model.coordinate_descent.enet_path arg mismatch: ['params'] (Lasso)
E           sklearn.linear_model.coordinate_descent.enet_path arg mismatch: ['params'] (MultiTaskElasticNet)
E           sklearn.linear_model.coordinate_descent.enet_path arg mismatch: ['params'] (MultiTaskElasticNetCV)
E           sklearn.linear_model.coordinate_descent.enet_path arg mismatch: ['params'] (MultiTaskLasso)
E           sklearn.linear_model.coordinate_descent.lasso_path arg mismatch: ['params']
E           sklearn.linear_model.coordinate_descent.lasso_path arg mismatch: ['params'] (LassoCV)
E           sklearn.linear_model.coordinate_descent.lasso_path arg mismatch: ['params'] (MultiTaskLassoCV)
E           sklearn.preprocessing.data.inverse_transform arg mismatch: ['X'] (QuantileTransformer)
E           sklearn.preprocessing.data.transform arg mismatch: ['copy'] (Binarizer)
E           sklearn.preprocessing.data.transform arg mismatch: ['copy'] (Normalizer)
E           sklearn.preprocessing.data.transform arg mismatch: ['copy'] (StandardScaler)

jnothman

The thing that's missing here (but can be in a future PR) is advice to developers (for the main project and for contrib). In general, we have some updates to issue contrib (including changing dependency versions).

Otherwise, LGTM on the basis of a fairly superficial review.

jnothman · 2017-07-09T21:22:07Z

flake8

./sklearn/tests/test_docstring_parameters.py:19:1: F401 'sklearn.utils.testing._get_func_name' imported but unused
from sklearn.utils.testing import _get_func_name
^
./sklearn/tests/test_docstring_parameters.py:127:21: F821 undefined name 'get_func_name'
            name_ = get_func_name(func)
                    ^
./sklearn/utils/testing.py:852:17: F821 undefined name 'get_func_name'
    func_name = get_func_name(func, class_name=class_name)
                ^

jnothman · 2017-07-09T21:22:44Z

Does that flake mean something isn't being run??

jnothman · 2017-07-09T23:10:56Z

sklearn/cluster/hierarchical.py

@@ -312,6 +312,9 @@ def linkage_tree(X, connectivity=None, n_components=None,
        be symmetric and only the upper triangular half is used.
        Default is None, i.e, the Ward algorithm is unstructured.

+    n_components : int (optional)


This parameter is unused. See #9307

(You don't want this addressed in this PR right?)

raghavrv · 2017-07-10T15:07:25Z

Let's merge this if you are happy? @jnothman @agramfort

agramfort · 2017-07-10T20:02:55Z

any idea why appveyor is not happy?

raghavrv · 2017-07-10T20:31:08Z

That seems to be because of #9317. should be fixed after that is merged and I update master. Or you can do it separately too. It should not break anything.

agramfort · 2017-07-10T21:37:47Z

please rebase so we merge a green PR

raghavrv · 2017-07-11T16:08:23Z

I think it's safe to merge without waiting for appveyor @agramfort (more delay leads to merge conflicts. especially after sprints)

agramfort · 2017-07-11T16:41:25Z

all green !

thx @raghavrv

jnothman · 2017-07-11T23:24:15Z

would one of you like to contribute this to numpydoc?

…

On 12 Jul 2017 2:42 am, "Alexandre Gramfort" ***@***.***> wrote: Merged #9206 <#9206>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9206 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEz6851EYeAqLjvIumEvNTk7OdY2ME1ks5sM6XlgaJpZM4ODclJ> .

…s and their order (scikit-learn#9206) * add automatic test of docstrings for function / method signatures using numpydoc

Release 0.19b2 * tag '0.19b2': (808 commits) Preparing 0.19b2 [MRG+1] FIX out of bounds array access in SAGA (scikit-learn#9376) FIX make test_importances pass on 32 bit linux Release 0.19b1 DOC remove 'in dev' header in whats_new.rst DOC typos in whats_news.rst [ci skip] [MRG] DOC cleaning up what's new for 0.19 (scikit-learn#9252) FIX t-SNE memory usage and many other optimizer issues (scikit-learn#9032) FIX broken link in gallery and bad title rendering [MRG] DOC Replace \acute by prime (scikit-learn#9332) Fix typos (scikit-learn#9320) [MRG + 1 (rv) + 1 (alex) + 1] Add a check to test the docstring params and their order (scikit-learn#9206) DOC Residual sum vs. regression sum (scikit-learn#9314) [MRG] [HOTFIX] Fix capitalization in test and hence fix failing travis at master (scikit-learn#9317) More informative error message for classification metrics given regression output (scikit-learn#9275) [MRG] COSMIT Remove unused parameters in private functions (scikit-learn#9310) [MRG+1] Ridgecv normalize (scikit-learn#9302) [MRG + 2] ENH Allow `cross_val_score`, `GridSearchCV` et al. to evaluate on multiple metrics (scikit-learn#7388) Add data_home parameter to fetch_kddcup99 (scikit-learn#9289) FIX makedirs(..., exists_ok) not available in Python 2 (scikit-learn#9284) ...

* releases: (808 commits) Preparing 0.19b2 [MRG+1] FIX out of bounds array access in SAGA (scikit-learn#9376) FIX make test_importances pass on 32 bit linux Release 0.19b1 DOC remove 'in dev' header in whats_new.rst DOC typos in whats_news.rst [ci skip] [MRG] DOC cleaning up what's new for 0.19 (scikit-learn#9252) FIX t-SNE memory usage and many other optimizer issues (scikit-learn#9032) FIX broken link in gallery and bad title rendering [MRG] DOC Replace \acute by prime (scikit-learn#9332) Fix typos (scikit-learn#9320) [MRG + 1 (rv) + 1 (alex) + 1] Add a check to test the docstring params and their order (scikit-learn#9206) DOC Residual sum vs. regression sum (scikit-learn#9314) [MRG] [HOTFIX] Fix capitalization in test and hence fix failing travis at master (scikit-learn#9317) More informative error message for classification metrics given regression output (scikit-learn#9275) [MRG] COSMIT Remove unused parameters in private functions (scikit-learn#9310) [MRG+1] Ridgecv normalize (scikit-learn#9302) [MRG + 2] ENH Allow `cross_val_score`, `GridSearchCV` et al. to evaluate on multiple metrics (scikit-learn#7388) Add data_home parameter to fetch_kddcup99 (scikit-learn#9289) FIX makedirs(..., exists_ok) not available in Python 2 (scikit-learn#9284) ...

* dfsg: (808 commits) Preparing 0.19b2 [MRG+1] FIX out of bounds array access in SAGA (scikit-learn#9376) FIX make test_importances pass on 32 bit linux Release 0.19b1 DOC remove 'in dev' header in whats_new.rst DOC typos in whats_news.rst [ci skip] [MRG] DOC cleaning up what's new for 0.19 (scikit-learn#9252) FIX t-SNE memory usage and many other optimizer issues (scikit-learn#9032) FIX broken link in gallery and bad title rendering [MRG] DOC Replace \acute by prime (scikit-learn#9332) Fix typos (scikit-learn#9320) [MRG + 1 (rv) + 1 (alex) + 1] Add a check to test the docstring params and their order (scikit-learn#9206) DOC Residual sum vs. regression sum (scikit-learn#9314) [MRG] [HOTFIX] Fix capitalization in test and hence fix failing travis at master (scikit-learn#9317) More informative error message for classification metrics given regression output (scikit-learn#9275) [MRG] COSMIT Remove unused parameters in private functions (scikit-learn#9310) [MRG+1] Ridgecv normalize (scikit-learn#9302) [MRG + 2] ENH Allow `cross_val_score`, `GridSearchCV` et al. to evaluate on multiple metrics (scikit-learn#7388) Add data_home parameter to fetch_kddcup99 (scikit-learn#9289) FIX makedirs(..., exists_ok) not available in Python 2 (scikit-learn#9284) ...

…s and their order (scikit-learn#9206) * add automatic test of docstrings for function / method signatures using numpydoc

agramfort and others added 25 commits June 9, 2017 10:06

add test script to have docstrings consistent with function signatures

3ae41ff

fix docstrings in datasets

3a1b925

add tests

04da883

update travis

00ebcfc

refactor

0b2c0eb

make travis happy?

5495e0e

do not crash when y=None for API reason

f3cefca

more fixes

a755044

more on metrics module

9fa05d8

more

f3d9c11

more

42b2a2d

more

98686e4

more

46abcd8

more

27936f4

more

88c71f3

more

c0a0630

more

d48d425

more

cd36d2d

more

4183f6a

more

a15459d

more

9a6f01d

Merge master

ed1a350

Merge pull request scikit-learn#15 from raghavrv/test_docstring_params

48f7433

Update with master and fix merge conflicts

Merge branch 'master' into test_docstring

b93b4cc

Merge branch 'master' into test_docstring

440231f

raghavrv mentioned this pull request Jun 23, 2017

[WIP/RFC] Test docstring parameters (with order) #9023

Closed

raghavrv added 3 commits June 23, 2017 13:46

No need of TEST_DOCSTRING='false'

d098674

Flake8 and licence

a1bbd53

Add Alex's name to the authors

b092418

jnothman reviewed Jul 9, 2017

View reviewed changes

jnothman changed the title ~~[MRG + 1 (rv) + 1 (alex)] Add a check to test the docstring params and their order~~ [MRG + 1 (rv) + 1 (alex) + 1] Add a check to test the docstring params and their order Jul 9, 2017

jnothman reviewed Jul 9, 2017

View reviewed changes

ColCarroll mentioned this pull request Jul 10, 2017

Numpy style docstrings? pymc-devs/pymc#2403

Closed

Address the rest of the comments

7e3e2ca

Merge branch 'master' into test_docstring

ed2f75d

raghavrv mentioned this pull request Jul 11, 2017

Make parameter docstrings match function/constuctor signatures #9325

Closed

9 tasks

agramfort merged commit b6f8865 into scikit-learn:master Jul 11, 2017

raghavrv deleted the test_docstring branch July 11, 2017 16:51

jnothman mentioned this pull request Jul 13, 2017

nosetesting modules for adherance to numpydoc docstring standards? numpy/numpydoc#13

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG + 1 (rv) + 1 (alex) + 1] Add a check to test the docstring params and their order #9206

[MRG + 1 (rv) + 1 (alex) + 1] Add a check to test the docstring params and their order #9206

raghavrv commented Jun 23, 2017 •

edited

Loading

raghavrv commented Jun 23, 2017 •

edited

Loading

jnothman left a comment

jnothman commented Jul 9, 2017

jnothman commented Jul 9, 2017

jnothman Jul 9, 2017

raghavrv Jul 10, 2017

raghavrv commented Jul 10, 2017

agramfort commented Jul 10, 2017

raghavrv commented Jul 10, 2017

agramfort commented Jul 10, 2017 via email

raghavrv commented Jul 11, 2017

agramfort commented Jul 11, 2017

jnothman commented Jul 11, 2017 via email

[MRG + 1 (rv) + 1 (alex) + 1] Add a check to test the docstring params and their order #9206

[MRG + 1 (rv) + 1 (alex) + 1] Add a check to test the docstring params and their order #9206

Conversation

raghavrv commented Jun 23, 2017 • edited Loading

raghavrv commented Jun 23, 2017 • edited Loading

jnothman left a comment

Choose a reason for hiding this comment

jnothman commented Jul 9, 2017

jnothman commented Jul 9, 2017

jnothman Jul 9, 2017

Choose a reason for hiding this comment

raghavrv Jul 10, 2017

Choose a reason for hiding this comment

raghavrv commented Jul 10, 2017

agramfort commented Jul 10, 2017

raghavrv commented Jul 10, 2017

agramfort commented Jul 10, 2017 via email

raghavrv commented Jul 11, 2017

agramfort commented Jul 11, 2017

jnothman commented Jul 11, 2017 via email

raghavrv commented Jun 23, 2017 •

edited

Loading

raghavrv commented Jun 23, 2017 •

edited

Loading