Skip to content

[MRG+1] FIX Add missing mixins to ClassifierChain #9473

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

Merged
merged 2 commits into from
Aug 4, 2017

Conversation

jnothman
Copy link
Member

@jnothman jnothman commented Aug 1, 2017

These shouldn't have been missing...

@jnothman jnothman added this to the 0.19 milestone Aug 1, 2017
@@ -380,6 +381,8 @@ def test_classifier_chain_fit_and_predict_with_logistic_regression():
assert_equal([c.coef_.size for c in classifier_chain.estimators_],
list(range(X.shape[1], X.shape[1] + Y.shape[1])))

assert isinstance(classifier_chain, ClassifierMixin)
Copy link
Member

Choose a reason for hiding this comment

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

In light of #9475, you should probably use is_classifier

Copy link
Member Author

Choose a reason for hiding this comment

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

Not in a test assertion. The point here is that it also ensures a default implementation is score

Copy link
Member

Choose a reason for hiding this comment

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

fair enough

@TomDLT TomDLT changed the title [MRG] FIX Add missing mixins to ClassifierChain [MRG+1] FIX Add missing mixins to ClassifierChain Aug 4, 2017
Copy link
Member

@TomDLT TomDLT left a comment

Choose a reason for hiding this comment

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

LGTM

@amueller amueller merged commit e674f64 into scikit-learn:master Aug 4, 2017
@amueller
Copy link
Member

amueller commented Aug 4, 2017

Hm maybe this kind of issue could be avoided with the estimator tags to check that tags are complete? hm...

@jnothman
Copy link
Member Author

jnothman commented Aug 6, 2017 via email

jnothman added a commit to jnothman/scikit-learn that referenced this pull request Aug 6, 2017
* Add missing mixins to ClassifierChain

* Fix import in test
dmohns pushed a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017
* Add missing mixins to ClassifierChain

* Fix import in test
dmohns pushed a commit to dmohns/scikit-learn that referenced this pull request Aug 7, 2017
* Add missing mixins to ClassifierChain

* Fix import in test
yarikoptic added a commit to yarikoptic/scikit-learn that referenced this pull request Aug 12, 2017
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 (scikit-learn#9527)
  sync whatsnew with master so I'm less confused
  DOC more navigation links
  DOC a note on data leakage and pipeline (scikit-learn#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 (scikit-learn#9482)
  Fix safe_indexing with read-only indices (scikit-learn#9507)
  [MRG+1] add scorer based on explained_variance_score (scikit-learn#9259)
  fix wrong assert in test_validation (scikit-learn#9480)
  [MRG+1] FIX Add missing mixins to ClassifierChain (scikit-learn#9473)
  Bring last code block in line with the image. (scikit-learn#9488)
  FIX Pass sample_weight as kwargs in VotingClassifier (scikit-learn#9493)
  FIX Incorrent implementation of noise_variance_ in PCA._fit_truncated (scikit-learn#9108)
  ...
yarikoptic added a commit to yarikoptic/scikit-learn that referenced this pull request Aug 12, 2017
* 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 (scikit-learn#9527)
  sync whatsnew with master so I'm less confused
  DOC more navigation links
  DOC a note on data leakage and pipeline (scikit-learn#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 (scikit-learn#9482)
  Fix safe_indexing with read-only indices (scikit-learn#9507)
  [MRG+1] add scorer based on explained_variance_score (scikit-learn#9259)
  fix wrong assert in test_validation (scikit-learn#9480)
  [MRG+1] FIX Add missing mixins to ClassifierChain (scikit-learn#9473)
  Bring last code block in line with the image. (scikit-learn#9488)
  FIX Pass sample_weight as kwargs in VotingClassifier (scikit-learn#9493)
  FIX Incorrent implementation of noise_variance_ in PCA._fit_truncated (scikit-learn#9108)
  ...
yarikoptic added a commit to yarikoptic/scikit-learn that referenced this pull request Aug 12, 2017
* 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 (scikit-learn#9527)
  sync whatsnew with master so I'm less confused
  DOC more navigation links
  DOC a note on data leakage and pipeline (scikit-learn#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 (scikit-learn#9482)
  Fix safe_indexing with read-only indices (scikit-learn#9507)
  [MRG+1] add scorer based on explained_variance_score (scikit-learn#9259)
  fix wrong assert in test_validation (scikit-learn#9480)
  [MRG+1] FIX Add missing mixins to ClassifierChain (scikit-learn#9473)
  Bring last code block in line with the image. (scikit-learn#9488)
  FIX Pass sample_weight as kwargs in VotingClassifier (scikit-learn#9493)
  FIX Incorrent implementation of noise_variance_ in PCA._fit_truncated (scikit-learn#9108)
  ...
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
* Add missing mixins to ClassifierChain

* Fix import in test
AishwaryaRK pushed a commit to AishwaryaRK/scikit-learn that referenced this pull request Aug 29, 2017
* Add missing mixins to ClassifierChain

* Fix import in test
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
* Add missing mixins to ClassifierChain

* Fix import in test
jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017
* Add missing mixins to ClassifierChain

* Fix import in test
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.

3 participants