[MRG+1] Common test refactoring by amueller · Pull Request #4550 · scikit-learn/scikit-learn

amueller · 2015-04-08T18:43:58Z

Introduces a public check_estimator that runs all checks.
Currently it relies on inheritance to see which tests to run (in addition to the general tests).
It could also rely on the recently introduced tags #4418 but there is currently only a single tag, while something might be a classifier and a transformer....

Todo:

tests for high-level interface function.
make sure all tests are run as they were before
add to docs

coveralls · 2015-04-08T18:58:22Z

Coverage decreased (-0.95%) to 94.2% when pulling 6211e07 on amueller:common_test_refactoring into 46aacd3 on scikit-learn:master.

coveralls · 2015-04-08T19:19:09Z

Coverage increased (+0.01%) to 95.17% when pulling 6211e07 on amueller:common_test_refactoring into 46aacd3 on scikit-learn:master.

amueller · 2015-04-08T19:36:10Z

sklearn/tests/test_common.py

this is absorbed into the tests above

amueller · 2015-04-09T16:50:22Z

sklearn/utils/tests/test_estimator_checks.py

This is ugly, and we could probably change that by raising an error. The orginal traceback is printed anyhow. @GaelVaroquaux thoughts?

agramfort · 2015-04-19T18:07:17Z

sklearn/utils/estimator_checks.py

shall it be done in this PR?

that is old, don't feel like doing it now ;)

agramfort · 2015-04-19T18:29:04Z

sent you a PR with nitpicks.

besides LGTM !

agramfort · 2015-04-19T19:21:30Z

travis is not happy :(

amueller · 2015-04-28T19:46:56Z

I need to rebase on #4590 to get consistent behavior for python2 and python3.

…aping.

…python3 version.

amueller · 2015-04-28T22:02:03Z

that should do it. that was some annoying testing....

amueller · 2015-04-28T22:04:40Z

agramfort · 2015-04-29T07:29:16Z

+1 for merge. thx @amueller !

amueller · 2015-04-29T20:16:41Z

thanks @agramfort for the review :)

ogrisel · 2015-04-30T11:16:01Z

This looks good to me. We can always improve further but I would rather not delay this refactoring that is already a net benefit.

[MRG+1] Common test refactoring

arjoly · 2015-04-30T12:04:10Z

Thanks @amueller !!!

mblondel · 2015-04-30T12:42:47Z

Great, I will try check_estimator on lightning when I get time.

mblondel · 2015-04-30T12:44:10Z

sklearn/utils/estimator_checks.py

Why not using is_classifier? (which is based on estimator tags rather than inheritance)

I commented somewhere: you can inherit from multiple mixins, but only have one tag currently. Maybe that is an issue with the tag system?
I could have used the tags for everything but Transformer, maybe I should have done that. I kind of punted on this one.

For detecting transformers, I think checking for fit_transform or transform should be pretty reliable. Not sure if we need to add a new is_transformer helper, though.

amueller · 2015-04-30T13:41:32Z

@mblondel yeah it would be sweet if you could try it. And I'm sure it will fail ;)

amueller · 2015-04-30T13:44:15Z

@ogrisel any improvment you have in mind / anything that is missing? thanks for the merge :)

mblondel · 2015-04-30T13:55:02Z

@amueller And lightning is probably far from compliant, especially w.r.t. to input checking.

amueller added this to the 1.0 milestone Apr 8, 2015

amueller reviewed Apr 8, 2015
View reviewed changes

sklearn/tests/test_common.py

Copy link
Copy Markdown

Member Author

amueller Apr 8, 2015

Choose a reason for hiding this comment

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

this is absorbed into the tests above

amueller force-pushed the common_test_refactoring branch from a13715a to c4f2488 Compare April 8, 2015 19:44

amueller changed the title ~~WIP Common test refactoring~~ [MRG] Common test refactoring Apr 8, 2015

amueller mentioned this pull request Apr 8, 2015

Estimator verification API #3810

Closed

amueller reviewed Apr 9, 2015
View reviewed changes

amueller mentioned this pull request Apr 17, 2015

Pickle tests have bad coverage #4614

Closed

agramfort reviewed Apr 19, 2015
View reviewed changes

amueller force-pushed the common_test_refactoring branch from ce5713f to ac56fbf Compare April 19, 2015 18:29

agramfort mentioned this pull request Apr 19, 2015

[MRG + 1] BUG: get_param in FeatureUnion disjoint for shallow/deep params #4467

Merged

amueller force-pushed the common_test_refactoring branch from 8205e36 to 4952874 Compare April 19, 2015 19:42

agramfort mentioned this pull request Apr 19, 2015

[MRG + 1] Add warning for KFold CV if random_state must be changed but shuffle is #4365

Closed

amueller force-pushed the common_test_refactoring branch from 4952874 to e386698 Compare April 19, 2015 20:02

amueller added 3 commits April 28, 2015 16:46

ENH add high-level estimator-validation function

669a4ea

check output, not error on sparse data handling

2d4c8d7

fix warning message by not using assert_raise_message, which adds esc…

196a7b7

…aping.

amueller force-pushed the common_test_refactoring branch from e386698 to 196a7b7 Compare April 28, 2015 20:46

amueller added 2 commits April 28, 2015 17:24

add comments to tests

8ecfa3c

FIX make backport of assert_raises_regex raise the same error as the …

14c54c4

…python3 version.

agramfort changed the title ~~[MRG] Common test refactoring~~ [MRG+1] Common test refactoring Apr 29, 2015

ogrisel added a commit that referenced this pull request Apr 30, 2015

Merge pull request #4550 from amueller/common_test_refactoring

b92ed8f

[MRG+1] Common test refactoring

ogrisel merged commit b92ed8f into scikit-learn:master Apr 30, 2015

ogrisel mentioned this pull request Apr 30, 2015

[RFC] Use explicit module levels skip lists for estimator checks #4649

Closed

mblondel reviewed Apr 30, 2015
View reviewed changes

amueller deleted the common_test_refactoring branch April 30, 2015 13:42

djsutherland mentioned this pull request Aug 18, 2015

use skl's common tests, as possible djsutherland/skl-groups#31

Open

amueller mentioned this pull request Sep 11, 2015

factorize common tests #406

Closed

Uh oh!

Conversation

amueller commented Apr 8, 2015

Uh oh!

coveralls commented Apr 8, 2015

Uh oh!

coveralls commented Apr 8, 2015

Uh oh!

amueller Apr 8, 2015

Choose a reason for hiding this comment

Uh oh!

amueller Apr 9, 2015

Choose a reason for hiding this comment

Uh oh!

agramfort Apr 19, 2015

Choose a reason for hiding this comment

Uh oh!

amueller Apr 19, 2015

Choose a reason for hiding this comment

Uh oh!

agramfort commented Apr 19, 2015

Uh oh!

agramfort commented Apr 19, 2015

Uh oh!

amueller commented Apr 28, 2015

Uh oh!

amueller commented Apr 28, 2015

Uh oh!

amueller commented Apr 28, 2015

Uh oh!

agramfort commented Apr 29, 2015

Uh oh!

amueller commented Apr 29, 2015

Uh oh!

ogrisel commented Apr 30, 2015

Uh oh!

arjoly commented Apr 30, 2015

Uh oh!

mblondel commented Apr 30, 2015

Uh oh!

mblondel Apr 30, 2015

Choose a reason for hiding this comment

Uh oh!

amueller Apr 30, 2015

Choose a reason for hiding this comment

Uh oh!

mblondel Apr 30, 2015

Choose a reason for hiding this comment

Uh oh!

amueller commented Apr 30, 2015

Uh oh!

amueller commented Apr 30, 2015

Uh oh!

mblondel commented Apr 30, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants