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+1] add scorer based on explained_variance_score #9259

Merged
merged 4 commits into from Aug 8, 2017

Conversation

Projects
None yet
3 participants
@qinhanmin2014
Member

qinhanmin2014 commented Jul 1, 2017

Reference Issue

Finish up #9183
proposed by @jnothman

What does this implement/fix? Explain your changes.

I change the regressor used in the test because the result of the regressor is so bad that make the test fail.

Any other comments?

irrelevant test failure due to #9254

@qinhanmin2014 qinhanmin2014 changed the title from [MRG] add scorer based on explained_variance_score to [WIP] add scorer based on explained_variance_score Jul 1, 2017

@qinhanmin2014 qinhanmin2014 changed the title from [WIP] add scorer based on explained_variance_score to [MRG] add scorer based on explained_variance_score Jul 1, 2017

Show outdated Hide outdated doc/modules/model_evaluation.rst
@@ -64,7 +64,7 @@
def _make_estimators(X_train, y_train, y_ml_train):
# Make estimators that make sense to test various scoring methods
sensible_regr = DummyRegressor(strategy='median')
sensible_regr = DecisionTreeRegressor(random_state=0)

This comment has been minimized.

@jnothman

jnothman Jul 2, 2017

Member

how much longer does the test take?

@jnothman

jnothman Jul 2, 2017

Member

how much longer does the test take?

This comment has been minimized.

@qinhanmin2014

qinhanmin2014 Jul 2, 2017

Member

@jnothman Thanks. After modification, the test takes around 0.1s on my PC, only slightly longer than before. In Travis, the test is not among the most time consuming tests. For the default parameter of random forest, n_estimators(10) is small, considering the number of training samples(100) is also small, the execution time seems to be acceptable.

@qinhanmin2014

qinhanmin2014 Jul 2, 2017

Member

@jnothman Thanks. After modification, the test takes around 0.1s on my PC, only slightly longer than before. In Travis, the test is not among the most time consuming tests. For the default parameter of random forest, n_estimators(10) is small, considering the number of training samples(100) is also small, the execution time seems to be acceptable.

@qinhanmin2014

This comment has been minimized.

Show comment
Hide comment
@qinhanmin2014

qinhanmin2014 Jul 2, 2017

Member

@jnothman Thanks. Corrected.

Member

qinhanmin2014 commented Jul 2, 2017

@jnothman Thanks. Corrected.

@qinhanmin2014

This comment has been minimized.

Show comment
Hide comment
@qinhanmin2014

qinhanmin2014 Aug 6, 2017

Member

ping @jnothman
Could you please give a further review? Thanks.

Member

qinhanmin2014 commented Aug 6, 2017

ping @jnothman
Could you please give a further review? Thanks.

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Aug 6, 2017

Member

LGTM

Member

jnothman commented Aug 6, 2017

LGTM

@jnothman jnothman changed the title from [MRG] add scorer based on explained_variance_score to [MRG+1] add scorer based on explained_variance_score Aug 6, 2017

@qinhanmin2014

This comment has been minimized.

Show comment
Hide comment
@qinhanmin2014

qinhanmin2014 Aug 6, 2017

Member

ping @lesteve @amueller
Could you kindly please have a look at it? (already [MRG+1])

Member

qinhanmin2014 commented Aug 6, 2017

ping @lesteve @amueller
Could you kindly please have a look at it? (already [MRG+1])

@lesteve

This comment has been minimized.

Show comment
Hide comment
@lesteve

lesteve Aug 7, 2017

Member

Can you add an entry in doc/whats_new.rst? Otherwise LGTM.

Member

lesteve commented Aug 7, 2017

Can you add an entry in doc/whats_new.rst? Otherwise LGTM.

@qinhanmin2014

This comment has been minimized.

Show comment
Hide comment
@qinhanmin2014

qinhanmin2014 Aug 8, 2017

Member

@jnothman @lesteve Thanks. What's new updated. CIs are green.

Member

qinhanmin2014 commented Aug 8, 2017

@jnothman @lesteve Thanks. What's new updated. CIs are green.

@lesteve

This comment has been minimized.

Show comment
Hide comment
@lesteve

lesteve Aug 8, 2017

Member

LGTM, merging, thanks a lot!

Member

lesteve commented Aug 8, 2017

LGTM, merging, thanks a lot!

@lesteve lesteve merged commit 9f91ec7 into scikit-learn:master Aug 8, 2017

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 96.16%)
Details
codecov/project 96.16% (+0%) compared to 6d4ae1b
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jnothman jnothman referenced this pull request Aug 8, 2017

Merged

RELEASE 0.19 #9502

jnothman added a commit to jnothman/scikit-learn that referenced this pull request Aug 8, 2017

yarikoptic added a commit to yarikoptic/scikit-learn that referenced this pull request Aug 12, 2017

Merge tag '0.19.0' into releases
Release 0.19.0

* tag '0.19.0': (99 commits)
  DOC one more version issue in doc
  skip docstring tests because not useful to users and has some issues
  deprecation of n_components happened in 0.19 not 0.18 (#9527)
  sync whatsnew with master so I'm less confused
  DOC more navigation links
  DOC a note on data leakage and pipeline (#9510)
  DOC set release date to Friday
  DOC Update news and menu for 0.19 release
  DOC list of contributors to 0.19
  DOC Change release date to Thursday
  DOC Remove some whitespace from what's new
  Update what's new for recent changes
  Use base.is_classifier instead instead of isinstance (#9482)
  Fix safe_indexing with read-only indices (#9507)
  [MRG+1] add scorer based on explained_variance_score (#9259)
  fix wrong assert in test_validation (#9480)
  [MRG+1] FIX Add missing mixins to ClassifierChain (#9473)
  Bring last code block in line with the image. (#9488)
  FIX Pass sample_weight as kwargs in VotingClassifier (#9493)
  FIX Incorrent implementation of noise_variance_ in PCA._fit_truncated (#9108)
  ...

yarikoptic added a commit to yarikoptic/scikit-learn that referenced this pull request Aug 12, 2017

Merge branch 'releases' into dfsg
* releases: (99 commits)
  DOC one more version issue in doc
  skip docstring tests because not useful to users and has some issues
  deprecation of n_components happened in 0.19 not 0.18 (#9527)
  sync whatsnew with master so I'm less confused
  DOC more navigation links
  DOC a note on data leakage and pipeline (#9510)
  DOC set release date to Friday
  DOC Update news and menu for 0.19 release
  DOC list of contributors to 0.19
  DOC Change release date to Thursday
  DOC Remove some whitespace from what's new
  Update what's new for recent changes
  Use base.is_classifier instead instead of isinstance (#9482)
  Fix safe_indexing with read-only indices (#9507)
  [MRG+1] add scorer based on explained_variance_score (#9259)
  fix wrong assert in test_validation (#9480)
  [MRG+1] FIX Add missing mixins to ClassifierChain (#9473)
  Bring last code block in line with the image. (#9488)
  FIX Pass sample_weight as kwargs in VotingClassifier (#9493)
  FIX Incorrent implementation of noise_variance_ in PCA._fit_truncated (#9108)
  ...

yarikoptic added a commit to yarikoptic/scikit-learn that referenced this pull request Aug 12, 2017

Merge branch 'dfsg' into debian
* dfsg: (99 commits)
  DOC one more version issue in doc
  skip docstring tests because not useful to users and has some issues
  deprecation of n_components happened in 0.19 not 0.18 (#9527)
  sync whatsnew with master so I'm less confused
  DOC more navigation links
  DOC a note on data leakage and pipeline (#9510)
  DOC set release date to Friday
  DOC Update news and menu for 0.19 release
  DOC list of contributors to 0.19
  DOC Change release date to Thursday
  DOC Remove some whitespace from what's new
  Update what's new for recent changes
  Use base.is_classifier instead instead of isinstance (#9482)
  Fix safe_indexing with read-only indices (#9507)
  [MRG+1] add scorer based on explained_variance_score (#9259)
  fix wrong assert in test_validation (#9480)
  [MRG+1] FIX Add missing mixins to ClassifierChain (#9473)
  Bring last code block in line with the image. (#9488)
  FIX Pass sample_weight as kwargs in VotingClassifier (#9493)
  FIX Incorrent implementation of noise_variance_ in PCA._fit_truncated (#9108)
  ...

paulha added a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017

AishwaryaRK added a commit to AishwaryaRK/scikit-learn that referenced this pull request Aug 29, 2017

maskani-moh added a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017

jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment